Re: [Emacs-diffs] emacs-25 b6d6304: Comment on last change to define-derived-mode

Re: [Emacs-diffs] emacs-25 b6d6304: Comment on last change to define-derived-mode

[Stefan Monnier]

> -	   (doc-string 4))
> +	   (doc-string 4)
> +	   ;; Ask not what
> +	   ;;(indent 3)
> +	   ;; can do for you, ask what it can do to others. IOW, the
> +	   ;; missing of indentation setting here is the indentation
> +	   ;; setting and not an oversight.
> +	   )
 
That should at least give one example where adding (indent 3) hurts.


        Stefan

Re: [Emacs-diffs] emacs-25 b6d6304: Comment on last change to define-derived-mode

[Leo Liu]

On 2016-03-01 09:24 -0500, Stefan Monnier wrote:
> That should at least give one example where adding (indent 3) hurts.

define-derived-mode is available since at least emacs 22.1 without any
(indent N) settings. I think maybe it is better to think in the other
direction that is what does (indent 3) bring to the table? is it worth
breaking 10 years of practice? Personally I have code that looks like
this:

 (define-derived-mode lcnt-mode special-mode nil :abbrev-table nil
   ...)

If you insist I'll append that example to the comment.

Thanks,
Leo

Re: [Emacs-diffs] emacs-25 b6d6304: Comment on last change to define-derived-mode

[Oleh Krehel]

Leo Liu <sdl.web@gmail.com> writes:

> On 2016-03-01 09:24 -0500, Stefan Monnier wrote:
>> That should at least give one example where adding (indent 3) hurts.
>
> define-derived-mode is available since at least emacs 22.1 without any
> (indent N) settings. I think maybe it is better to think in the other
> direction that is what does (indent 3) bring to the table? is it worth
> breaking 10 years of practice? Personally I have code that looks like
> this:
>
>  (define-derived-mode lcnt-mode special-mode nil :abbrev-table nil
>    ...)

This is a wrong kind of code in my opinion. There is value in all code
looking the same and feeling immediately right to every programmer.  If
in 95% of cases `define-derived-mode' already looks like it is (indent
3), why not make that "the official look" and nudge the remaining 5%
(i.e. you) into conforming.

This is the kind of thought that I've put in to making that commit. I
actually examined all instances of indentation in core and MELPA.

Unfortunately, we have a person that puts his own fancies ahead of the
better style for everyone. And has the audacity to revert commits just
because his little bit of code doesn't look right.

    Oleh

Re: [Emacs-diffs] emacs-25 b6d6304: Comment on last change to define-derived-mode

[Kaushal Modi]

[-- Attachment #1: Type: text/plain, Size: 535 bytes --]

That style of writing everything in a single line will usually cause that
line to exceed the default fill-column of 70. I believe most of the code in
emacs source tries to keep the line lengths around that number.

Also I am one of those people who prefer keeping line lengths around 70-80
characters. The 3-space indentation style helps achieve that goal. IMO
breaking up forms like define-derived-mode, define-minor-mode, etc makes
the code easier to read if each key-value component is written on a
different line.

--
Kaushal Modi

[-- Attachment #2: Type: text/html, Size: 594 bytes --]

Re: [Emacs-diffs] emacs-25 b6d6304: Comment on last change to define-derived-mode

[Stefan Monnier]

>> That should at least give one example where adding (indent 3) hurts.
> define-derived-mode is available since at least emacs 22.1 without any
> (indent N) settings. I think maybe it is better to think in the other
> direction that is what does (indent 3) bring to the table?

That's irrelevant to the issue of having to documenting *why* you
revert something.


        Stefan

Re: [Emacs-diffs] emacs-25 b6d6304: Comment on last change to define-derived-mode

[Dmitry Gutov]

On 03/04/2016 04:26 PM, Stefan Monnier wrote:
>>> That should at least give one example where adding (indent 3) hurts.
>> define-derived-mode is available since at least emacs 22.1 without any
>> (indent N) settings. I think maybe it is better to think in the other
>> direction that is what does (indent 3) bring to the table?
>
> That's irrelevant to the issue of having to documenting *why* you
> revert something.

Putting a code sample into the comment right there would be pretty 
awkward, though. I'd rather see a message to emacs-devel or a bug 
report, and a link to either in the reverting commit.

In this case, though, maybe we should revert the revert, all things 
considered.

Re: [Emacs-diffs] emacs-25 b6d6304: Comment on last change to define-derived-mode

[Leo Liu]

On 2016-03-04 18:10 +0200, Dmitry Gutov wrote:
> In this case, though, maybe we should revert the revert, all things
> considered.

I love to hear the `all things' that you have considered? Make a trivial
change is trivial but I would like to know we have enough justification
to change something that people have grown dependent in the past 10
years. Care to elaborate?

I am tired of people claiming representing 90ish% users when there is
only just one or two (at most a handful) disregard the silent majority.

No users will come and praise how good some feature is. What we know is
nobody has complaint anything in the past 10 years. This is hard
evidence.

Leo

Re: [Emacs-diffs] emacs-25 b6d6304: Comment on last change to define-derived-mode

[Dmitry Gutov]

On 03/05/2016 04:09 AM, Leo Liu wrote:
> On 2016-03-04 18:10 +0200, Dmitry Gutov wrote:
>> In this case, though, maybe we should revert the revert, all things
>> considered.
>
> I love to hear the `all things' that you have considered?

That putting the keyword arguments from BODY on the same line as the 
mode name is semantically wrong? And it's prone to result in longer 
lines? And that right now there's a weird comment inside 
define-derived-mode that makes little sense without additional context?

> Make a trivial
> change is trivial but I would like to know we have enough justification
> to change something that people have grown dependent in the past 10
> years.

It might cause you to reformat a few lines over the next few years, but 
it's hardly a cause for major concern.

However: I'm having hard time reproducing the justification stated in 
2c3b05c93e4884460068a12357ebd04ae4cd7446. The given example indents just 
as fine without (indent 3), in 'emacs -Q'.

Oleh, care to elaborate?

Re: [Emacs-diffs] emacs-25 b6d6304: Comment on last change to define-derived-mode

[Leo Liu]

On 2016-03-05 05:11 +0200, Dmitry Gutov wrote:
> That putting the keyword arguments from BODY on the same line as the
> mode name is semantically wrong? And it's prone to result in longer
> lines? And that right now there's a weird comment inside
> define-derived-mode that makes little sense without additional
> context?

I wouldn't consider the keywords part of BODY (they aren't eval'd as
they appear anyway). They actually affect how code is generated. The
BODY has at most 3 semantic parts: DOC, KEYWORDS and BODY2. The first 2
are optional. My style is not bizarre and to my eye make the code clear.
Everyone has own lisp style and we should do our best to respect them.

Leo

Re: [Emacs-diffs] emacs-25 b6d6304: Comment on last change to define-derived-mode

[Dmitry Gutov]

On 03/05/2016 11:48 AM, Leo Liu wrote:

> I wouldn't consider the keywords part of BODY (they aren't eval'd as
> they appear anyway).

Then we can consider them a separate argument. Since it's not delimited 
with parens, what better way to emphasize it than to put it on a 
separate line?

> They actually affect how code is generated. The
> BODY has at most 3 semantic parts: DOC, KEYWORDS and BODY2. The first 2
> are optional. My style is not bizarre and to my eye make the code clear.
> Everyone has own lisp style and we should do our best to respect them.

Not if we lose out in functionality in other places (still waiting for 
Oleh's response on that one).

Anyway, any user, yourself included, can override the 
`lisp-indent-function' property in their init file.

Re: [Emacs-diffs] emacs-25 b6d6304: Comment on last change to define-derived-mode

[Leo Liu]

On 2016-03-05 15:32 +0200, Dmitry Gutov wrote:
> Then we can consider them a separate argument. Since it's not
> delimited with parens, what better way to emphasize it than to put it
> on a separate line?

So what's your point of bringing up semantics in the first place? I am
totally lost.

>> They actually affect how code is generated. The BODY has at most 3
>> semantic parts: DOC, KEYWORDS and BODY2. The first 2 are optional. My
>> style is not bizarre and to my eye make the code clear. Everyone has
>> own lisp style and we should do our best to respect them.
>
> Not if we lose out in functionality in other places

Still missing the point. I don't mind if you exert some style in a new
macro. But this is a macro that has been there since at least 22.1.

Leo

Re: [Emacs-diffs] emacs-25 b6d6304: Comment on last change to define-derived-mode

[Dmitry Gutov]

On 03/05/2016 04:41 PM, Leo Liu wrote:

> So what's your point of bringing up semantics in the first place? I am
> totally lost.

Put keyword arguments on a separate line. It's their place, semantically.

> Still missing the point. I don't mind if you exert some style in a new
> macro. But this is a macro that has been there since at least 22.1.

Sometimes we improve existing facilities too, not just pile on new ones.

Re: [Emacs-diffs] emacs-25 b6d6304: Comment on last change to define-derived-mode

[Dmitry Gutov]

On 03/05/2016 03:56 PM, Oleh Krehel wrote:

> It indents just fine with `lisp-indent-function' but not with
> `common-lisp-indent-function'.

Not a problem, define-derived-mode is not a Common Lisp function.

> Still, (indent 3) is meta-data, which should be provided and is useful
> in any case (for example, I often use a pretty printer, and they can use
> the meta-data to insert raw Lisp code in a pretty way).

That might be a decent goal, but I don't think we generally take it into 
consideration. Unless I'm mistaken, it'll need a discussion first, and 
probably a bug report.

Re: [Emacs-diffs] emacs-25 b6d6304: Comment on last change to define-derived-mode

[John Wiegley]

>>>>> Leo Liu <sdl.web@gmail.com> writes:

> I love to hear the `all things' that you have considered? Make a trivial
> change is trivial but I would like to know we have enough justification to
> change something that people have grown dependent in the past 10 years. Care
> to elaborate?

I agree that defaults like this should not be changed without discussion.

Oleh, why was this change made back in January?

-- 
John Wiegley                  GPG fingerprint = 4710 CF98 AF9B 327B B80F
http://newartisans.com                          60E1 46C4 BD1A 7AC1 4BA2

Re: [Emacs-diffs] emacs-25 b6d6304: Comment on last change to define-derived-mode

[Oleh Krehel]

John Wiegley <jwiegley@gmail.com> writes:

> Oleh, why was this change made back in January?

I made the change because I had need for it, and estimated that it
should not affect /any/ code that was laid out properly.

Adding meta-data is an innocuous operation, just like adding
documentation where there was none before.

Re: [Emacs-diffs] emacs-25 b6d6304: Comment on last change to define-derived-mode

[John Wiegley]

>>>>> Oleh Krehel <oleh@oremacs.com> writes:

> Adding meta-data is an innocuous operation, just like adding documentation
> where there was none before.

Wouldn't it change the way old code might get re-indented?  In other words,
was this a no-op addition of metadata, or did it have aesthetic impact on the
way code gets indented?

-- 
John Wiegley                  GPG fingerprint = 4710 CF98 AF9B 327B B80F
http://newartisans.com                          60E1 46C4 BD1A 7AC1 4BA2

Re: [Emacs-diffs] emacs-25 b6d6304: Comment on last change to define-derived-mode

[Leo Liu]

On 2016-03-06 13:01 -0800, John Wiegley wrote:
> Wouldn't it change the way old code might get re-indented? In other
> words, was this a no-op addition of metadata, or did it have aesthetic
> impact on the way code gets indented?

I think it is a misunderstanding that missing a (indent N) in an
oversight when it is actually a deliberate design. A lot of the def-ish
macros have this pattern. See the doc-string of lisp-indent-function.

HTH,
Leo

Re: [Emacs-diffs] emacs-25 b6d6304: Comment on last change to define-derived-mode

[Oleh Krehel]

John Wiegley <jwiegley@gmail.com> writes:

>> Adding meta-data is an innocuous operation, just like adding documentation
>> where there was none before.
>
> Wouldn't it change the way old code might get re-indented?  In other words,
> was this a no-op addition of metadata, or did it have aesthetic impact on the
> way code gets indented?

A bit from column A, a bit from column B. I think it's a flaw in the
current `indent' metadata design that the absence of metadata is itself
metadata. Since it makes it impossible to distinguish whether the
absence of metadata is intentional or an oversight.

You can grep the core for "(define-derived-mode " and see the possible
effect for yourself. Here's a summary: out of 301 grep hits in *.el
files, 19 instances don't follow the (indent 3) rule - all the others
do:

    (define-derived-mode bookmark-edit-annotation-mode
    (define-derived-mode diary-fancy-display-mode special-mode
    (define-derived-mode semantic-grammar-mode
    (define-derived-mode electric-buffer-menu-mode Buffer-menu-mode
    (define-derived-mode reb-lisp-mode
    (define-derived-mode smime-mode fundamental-mode
    (define-derived-mode image-dired-thumbnail-mode
    (define-derived-mode image-dired-display-image-mode
    (define-derived-mode network-connection-mode
    (define-derived-mode newsticker-mode fundamental-mode
    (define-derived-mode newsticker-treeview-list-mode newsticker-treeview-mode
    (define-derived-mode newsticker-treeview-item-mode newsticker-treeview-mode
    (define-derived-mode ebrowse-electric-list-mode
    (define-derived-mode ebrowse-electric-position-mode
    (define-derived-mode elisp-byte-code-mode emacs-lisp-mode
    (define-derived-mode pages-directory-address-mode pages-directory-mode
    (define-derived-mode thumbs-mode
    (define-derived-mode thumbs-view-image-mode
    (define-derived-mode vc-git-region-history-mode

So I've added an indentation hint that 94% of the code already follows,
while 6% doesn't. It's up to you as the maintainer to decide if we want
to have a 94/6 split just to not ruffle any feathers. Or we want to
enforce a rule to make all code uniform - easier to read and contribute
to.

Personally I find it annoying tripping over things and trying to
preserve the ancient indentation when contributing to the Emacs core.
This could be a non-issue in the future.

Re: [Emacs-diffs] emacs-25 b6d6304: Comment on last change to define-derived-mode

[John Wiegley]

>>>>> Oleh Krehel <oleh@oremacs.com> writes:

> So I've added an indentation hint that 94% of the code already follows,
> while 6% doesn't. It's up to you as the maintainer to decide if we want to
> have a 94/6 split just to not ruffle any feathers. Or we want to enforce a
> rule to make all code uniform - easier to read and contribute to.

If we're formalizing what 94% of our code already does, that can helpful for
setting expected standards.  However, that's 94% of our code, not 94% of
everything that's out there.

-- 
John Wiegley                  GPG fingerprint = 4710 CF98 AF9B 327B B80F
http://newartisans.com                          60E1 46C4 BD1A 7AC1 4BA2

Re: [Emacs-diffs] emacs-25 b6d6304: Comment on last change to define-derived-mode

[Oleh Krehel]

John Wiegley <jwiegley@gmail.com> writes:

>>>>>> Oleh Krehel <oleh@oremacs.com> writes:
>
>> So I've added an indentation hint that 94% of the code already follows,
>> while 6% doesn't. It's up to you as the maintainer to decide if we want to
>> have a 94/6 split just to not ruffle any feathers. Or we want to enforce a
>> rule to make all code uniform - easier to read and contribute to.
>
> If we're formalizing what 94% of our code already does, that can helpful for
> setting expected standards.  However, that's 94% of our code, not 94% of
> everything that's out there.

Like I said, it's up to you to decide what to do.  You've already shown
favor to the testing and bug-squashing directions for the Emacs
development.

In my opinion, the coding style could be another useful direction. By
this I mean:

- Removing the ambiguity about `indent-tabs-mode'.

- Working on something like `pretty-print-buffer': a command to
  automatically eliminate hanging parens, consecutive spaces,
  multi-variable `setq'; and re-indent everything.
    
- Extending the checkdoc to find more warnings in the code style, like
  unresolved declarations etc.

- Formalizing the C code style, tutorials, contribution guidelines
  etc. Maybe we could have a linter for C.

I think it would be great if at some moment we could stop thinking how a
code looks, and only think about what it does. And ignoring how it looks
isn't really a great solution - it may work for some, doesn't work for
me at the moment.

Re: [Emacs-diffs] emacs-25 b6d6304: Comment on last change to define-derived-mode

[Clément Pit--Claudel]

[-- Attachment #1.1: Type: text/plain, Size: 135 bytes --]

On 03/07/2016 10:26 AM, Oleh Krehel wrote:
> multi-variable `setq';

Can you remind me of what's wrong with multi-variable setq?


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [Emacs-diffs] emacs-25 b6d6304: Comment on last change to define-derived-mode

[Oleh Krehel]

Clément Pit--Claudel <clement.pit@gmail.com> writes:

> On 03/07/2016 10:26 AM, Oleh Krehel wrote:
>> multi-variable `setq';
>
> Can you remind me of what's wrong with multi-variable setq?

It's error-prone and hard to read/manipulate.

RE: [Emacs-diffs] emacs-25 b6d6304: Comment on last change to define-derived-mode

[Drew Adams]

> >> multi-variable `setq';
> >
> > Can you remind me of what's wrong with multi-variable setq?
> 
> It's error-prone

Demonstration?

> and hard to read/manipulate.

Manipulate, maybe.  But no more than `let*' or `cond'.

Read?  Definitely not.  IMHO.

Re: [Emacs-diffs] emacs-25 b6d6304: Comment on last change to define-derived-mode

[Clément Pit--Claudel]

[-- Attachment #1.1: Type: text/plain, Size: 483 bytes --]

On 03/07/2016 11:03 AM, Drew Adams wrote:
>> > and hard to read/manipulate.
> Manipulate, maybe.  But no more than `let*' or `cond'.

Actually, its true that it's harder to swap the first and second assignment in a multi-variable setq than a multivariable let*, don't you think?

(setq a 1 
      b 2)

vs

(let* ((a 1)
       (b 2))
  ...)

> Read?  Definitely not.  IMHO.

Agreed: I use multi-variable setq a lot in my init files; I think it reads very nicely.


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

RE: [Emacs-diffs] emacs-25 b6d6304: Comment on last change to define-derived-mode

[Drew Adams]

> >> > and hard to read/manipulate.
> > Manipulate, maybe.  But no more than `let*' or `cond'.
> 
> Actually, its true that it's harder to swap the first and second
> assignment in a multi-variable setq than a multivariable let*, don't
> you think?
> (setq a 1
>       b 2)
> vs
> (let* ((a 1)
>        (b 2))
>   ...)

OK, yes, slightly "harder".  Big deal.

As I said, I weigh in on the side of code readability, not ease
of writing.  Probably most (at least many) programmers do the
opposite.

Either way, we are talking about _minor_ differences: in (a) writing
difficulty on the one hand and (b) reading difficulty on the other.

I favor the reader over the writer.  But I'm not writing/maintaining
this code.  I don't mind the minor editing chore for my own code,
but I can understand someone who just wants to get on with quickly
making changes and is not nit-picky about readability.

Personally, my impression is that (what I would call) overemphasis
on ease of writing is a trait of beginners, who can tend to think
that (1) they are writing only for themselves and (2) the code will
not live a long lifetime.  And then it can become a habit, so that
even experienced coders think/act like this.

> > Read?  Definitely not.  IMHO.
> 
> Agreed: I use multi-variable setq a lot in my init files; I think it
> reads very nicely.

I expect that we are in the minority, perhaps even a tiny minority.
But I don't know that.

RE: [Emacs-diffs] emacs-25 b6d6304: Comment on last change to define-derived-mode

[Drew Adams]

> > multi-variable `setq';
> 
> Can you remind me of what's wrong with multi-variable setq?

Nothing.  IMHO.

You could complain that it can require more maintenance, just like
`cond', `let*', and other sequential constructs.

Some people prioritize writing code over reading code.  To me this
is misguided, and multi-variable `setq' is a plus, not a minus.

FWIW, for a very long time I avoided multi-variable setq.  Dunno
why - probably just maintenance laziness.

Later I learned to appreciate it.  I also go to the trouble of
(manually) aligning the values (for readability), something that
I also do now for `let' and `let*'.

Re: [Emacs-diffs] emacs-25 b6d6304: Comment on last change to define-derived-mode

[Thierry Volpiatto]

Drew Adams <drew.adams@oracle.com> writes:

> Some people prioritize writing code over reading code.  To me this
> is misguided, and multi-variable `setq' is a plus, not a minus.

1+

Please don't modify this setq feature. For readability, note that eldoc
highlighting is showing you where your cursor is (var or value), so I
don't see where the problem is.

Thanks.

-- 
Thierry

Re: [Emacs-diffs] emacs-25 b6d6304: Comment on last change to define-derived-mode

[John Wiegley]

>>>>> Oleh Krehel <oleh@oremacs.com> writes:

> Like I said, it's up to you to decide what to do. You've already shown favor
> to the testing and bug-squashing directions for the Emacs development.
> 
> In my opinion, the coding style could be another useful direction.

I do think coding style can be of value, but I'd rate it at far lower
importance than testing and bug-squashing. The reason being that improving
tests and reducing bugs benefits all of our users, while coding styles mostly
benefit the people on this list.

> - Removing the ambiguity about `indent-tabs-mode'.

What is the ambiguity?

> - Working on something like `pretty-print-buffer': a command to
> automatically eliminate hanging parens, consecutive spaces, multi-variable
> `setq'; and re-indent everything.

This could be quite nice!

> - Extending the checkdoc to find more warnings in the code style, like
> unresolved declarations etc.

Agreed.

> - Formalizing the C code style, tutorials, contribution guidelines etc.
> Maybe we could have a linter for C.

Yes!

> I think it would be great if at some moment we could stop thinking how a
> code looks, and only think about what it does. And ignoring how it looks
> isn't really a great solution - it may work for some, doesn't work for me at
> the moment.

Personally I'd love that too. I'd just rather see the bugs under much better
control before putting too much energy in this direction, as its likely to
generate a lot of debate.

-- 
John Wiegley                  GPG fingerprint = 4710 CF98 AF9B 327B B80F
http://newartisans.com                          60E1 46C4 BD1A 7AC1 4BA2

Re: [Emacs-diffs] emacs-25 b6d6304: Comment on last change to define-derived-mode

[Oleh Krehel]

John Wiegley <jwiegley@gmail.com> writes:

>> - Removing the ambiguity about `indent-tabs-mode'.
>
> What is the ambiguity?

Allowing a mix of tabs and spaces in the core. I'd vote to have this
rule enforced:

    (setq indent-tabs-mode nil)

I think it was discussed previously by Dmitry, but that discussion was
stuck for a while now.

Re: [Emacs-diffs] emacs-25 b6d6304: Comment on last change to define-derived-mode

[Dmitry Gutov]

On 03/07/2016 05:58 PM, Oleh Krehel wrote:

> Allowing a mix of tabs and spaces in the core. I'd vote to have this
> rule enforced:
>
>     (setq indent-tabs-mode nil)
>
> I think it was discussed previously by Dmitry, but that discussion was
> stuck for a while now.

The core has been taken care of: see the top-level .dir-locals.el in the 
Emacs repository.

Judging by the previous discussions, it's highly unlikely that we'll get 
anything stronger than that.

Re: [Emacs-diffs] emacs-25 b6d6304: Comment on last change to define-derived-mode

[Stefan Monnier]

FWIW, I'm really sorry to have occasioned this longwinded
bike-shedding.  My *only* point was to ask to get some concrete example
where the patch was undesirable, so that we know concretely why the
change was undone.
I did not intend to put in doubt the validity of the reversion.


        Stefan


>>> That should at least give one example where adding (indent 3) hurts.
>> define-derived-mode is available since at least emacs 22.1 without any
>> (indent N) settings. I think maybe it is better to think in the other
>> direction that is what does (indent 3) bring to the table?

> That's irrelevant to the issue of having to documenting *why* you
> revert something.


>         Stefan

Re: [Emacs-diffs] emacs-25 b6d6304: Comment on last change to define-derived-mode

[Dmitry Gutov]

On 03/08/2016 06:35 AM, Stefan Monnier wrote:
> My *only* point was to ask to get some concrete example
> where the patch was undesirable, so that we know concretely why the
> change was undone.

Are we going to keep the newly added comment there, the one that doesn't 
provide the above information?