Codifying some aspects of Elisp code style and improving pretty printer

Codifying some aspects of Elisp code style and improving pretty printer

[akater]

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

Rgrepping lisp/emacs-lisp alone for “(let” and “(if” demonstrates that
there is a reasonable practice at least

- of keeping “then” on the same line as “if” sometimes

- of having several let bindings in “let” on the same line, especially
  if they are all symbols except maybe some first ones.

There is an informal consensus that it's worth it to use whitespace
wisely to keep Lisp forms concise vertically as well as horizontally.  I
think if the idea is recognised as useful it better be explicitly stated
as such rather than remain folklore. I don't see this practice codified
neither in elisp nor eintr Info nodes nor anywhere in Lisp literature
for that matter.  I asked in commonlisp channel on libera.chat and
nobody seems to know of any text like that; pointers are welcome if I
missed something.

True, this is largerly a matter of personal style.  However, there is
also some accumulated experience which I think is worth aggregating.
And no style is actually 100% personal when we collaborate.

I paid some attention to the issue for some time while writing (E)Lisp
code.  There are still some practices I'm not sure about.
E.g. apparently there's no practice to start the first subform of
unwind-protect on the same line as unwind-protect when line width
allows it; I find this surprising as I think this significantly
decreases readablity of unwind-protect forms.

What follows is a draft of relevant style guidelines as I see it.  At
the very least, it can serve as a basis for improving the currently
used pretty printer that prints Elisp forms (which still inexplicably
insists that let forms' bindings should start on a new line, for
example).  Automatic indentation should not attempt to be smart about
line splits but pretty printer should.

I'd welcome relevant bits of github.com/bbatsov/emacs-lisp-style-guide
as well but the guide mentions issues Emacs seems to deal on its own
automatically so I'm not sure.  Also, there may be copyright issues.

When it comes to accessing such information from within Emacs, I could
only find something in (recent) compiler warnings, about long
docstrings, and some other notes about docstrings in CONTRIBUTE file.
Since it has sporadic notes on docstring style, CONTRIBUTE likely
should also reference the style guide as well as Emacs Lisp Coding
Conventions Info node.

So here's the draft.

Emacs Lisp source code layout guidelines

- Rule of thumb: General form should look like

  (f x
     y
     z)

  However, if sequences f x1 ... xn, y1 ... ym, ..., z1 ... zk are
  short as a whole, it is preferred to

  (f x1 ... xn
     y1 ... ym
     ...
     z1 ... zk)

  especially if expressions on the same line are close conceptually,
  e.g. (a) key-value pairs, including those with keyword arguments, or
  (b) pairs of arguments in setq, setf and similar forms.

- “Short as a whole” necessarily means “fits into a single line of
  recommended line width, together with all opening parens” but this
  condition is not sufficient.

- In let, let* and similar forms, a single line can hold as many
  bindings represented by symbols rather than lists, as long as they
  are short as a whole.

- In let* and similar forms, a binding with initial value that depends
  solely on a previous (but not necessarily immediately previous)
  value, can be kept on the same line as its dependency as long as
  they are short as a whole.

- In let, let* and similar forms, if it is posible to arrange
  variables so that conceptually close ones share the same line, it is
  preferred to do so.

- In most (indent n) forms, it is preferred to start (n-1)th
  subexpression on the same line as the head symbol if the
  subexpressions in question are short as a whole and if it allows to
  keep the overall form concise both horizontally and vertically; note
  that it applies to forms ignore-errors and unwind-protect.

- The former applies to cl-loop, cond, let, if forms which do not have
  (indent ..) in their specs but which can nevertheless be considered
  for our purposes as (indent 0), (indent 0), (indent 1), (indent 2)
  forms correspondingly.

- Aside from the very first one, cl-loop keywords should start the lines.

- Two or more forms, short as a whole, that perform what's essentially
  a single step of a loop can be kept on the same line.

- “Short”, “most forms”, “similar forms” in the above paragraphs are
  subjective so the final decision on whether keeping sexps on the
  same line hampers readability or not is up to a maintainer of the
  Elisp codebase in question.  Arguing about such things with
  maintainers is likely a waste of time.  Thus, the maintainer's
  decision on line splits should be considered final.  Refactoring /
  cleanup edits that deal with line splits better be left to
  maintainers, or outsourced by them to someone whose personal style
  they explicitly trust enough to avoid any arguments about this
  matter.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 865 bytes --]

Re: Codifying some aspects of Elisp code style and improving pretty printer

[André A. Gomes]

akater <nuclearspace@gmail.com> writes:

> There is an informal consensus that it's worth it to use whitespace
> wisely to keep Lisp forms concise vertically as well as horizontally.  I
> think if the idea is recognised as useful it better be explicitly stated
> as such rather than remain folklore. [...]
>
> True, this is largerly a matter of personal style.  However, there is
> also some accumulated experience which I think is worth aggregating.
> And no style is actually 100% personal when we collaborate.

Akater, I think the topic you're raising is important.

My only thought is that it makes little sense to aggregate these
folklore syntax practices without a (heartless) linting tool that
actually enforces/checks if they're being respected.


-- 
André A. Gomes
"Free Thought, Free World"

Re: Codifying some aspects of Elisp code style and improving pretty printer

[Bozhidar Batsov]

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

Style discussions tend to quickly digress into endless bikeshedding, so be careful what you wish for. :-)

I've started the GitHub style guide project for 2 reasons:

- it's easier to discuss code snippets there (mostly because they are properly formatted)
- I wanted to avoid the extra work to update the Emacs manual

I hoped that eventually some of this work would go back to the Elisp manual, but lately I haven't had much time for the style guide and it has been somewhat dormant. 

I completely agree that in the end of the day one also needs to enforce style automatically (e.g. via formatters/linters). That's what I did with Ruby and RuboCop, but it's so much work that I didn't want to do it for other languages. :-) 

On Thu, Sep 30, 2021, at 10:02 AM, André A. Gomes wrote:
> akater <nuclearspace@gmail.com> writes:
> 
> > There is an informal consensus that it's worth it to use whitespace
> > wisely to keep Lisp forms concise vertically as well as horizontally.  I
> > think if the idea is recognised as useful it better be explicitly stated
> > as such rather than remain folklore. [...]
> >
> > True, this is largerly a matter of personal style.  However, there is
> > also some accumulated experience which I think is worth aggregating.
> > And no style is actually 100% personal when we collaborate.
> 
> Akater, I think the topic you're raising is important.
> 
> My only thought is that it makes little sense to aggregate these
> folklore syntax practices without a (heartless) linting tool that
> actually enforces/checks if they're being respected.
> 
> 
> -- 
> André A. Gomes
> "Free Thought, Free World"
> 
> 

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

Re: Codifying some aspects of Elisp code style and improving pretty printer

[akater]

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

"Bozhidar Batsov" <bozhidar@batsov.dev> writes:

> Style discussions tend to quickly digress into endless bikeshedding,
> so be careful what you wish for. :-)

One of the points of my suggestion is to ensure there'd be no such
discussions, at least in mailing lists / issue trackers.  In the absence
of both such rule and codified style practices, it leads to more
arguing, or at the very least temptation to.  I just had a patch merged
with such stylistic edits.  If there was a clear rule on such
discussions, that'd be it.  If there were stylistic guidelines, I would
adhere to them in the first place and there would be even less hassle.

But I don't think in Lisp this could be enforced in its entirety which
is why I said that indentation mechanism should not attempt to enforce
this.  It's very subjective.  Often, I want to keep a long line and a
couple of moments afterwards I decide that it's too long after all.

Lisp is very flexible even when it comes to code arrangement, and
personal taste matters a lot.  I don't think it can be automated.  Well,
maybe symbols in let bindings can be indented automatically to fill the
empty space but that's that, and even then, sometimes I arrange
interconncted variabls in groups, each on its own line.  A linter would
wreck that, in general.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 865 bytes --]

Re: Codifying some aspects of Elisp code style and improving pretty printer

[Lars Ingebrigtsen]

akater <nuclearspace@gmail.com> writes:

> But I don't think in Lisp this could be enforced in its entirety which
> is why I said that indentation mechanism should not attempt to enforce
> this.  It's very subjective.  Often, I want to keep a long line and a
> couple of moments afterwards I decide that it's too long after all.

A coding style that's not automatically enforced isn't very useful, in
my opinion.  It just leads to even more arguing, with some people
insisting that it should always be adhered to, and others arguing that
this is a special case where it shouldn't.

We (the maintainers) usually touch up code before committing to adhere
to our opinions of how the code should look, and hope that submitters
pick up on it after a while (so that we don't have to keep on doing it).

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Re: Codifying some aspects of Elisp code style and improving pretty printer

[Stefan Monnier]

> A coding style that's not automatically enforced isn't very useful, in
> my opinion.  It just leads to even more arguing, with some people
> insisting that it should always be adhered to, and others arguing that
> this is a special case where it shouldn't.

I guess there is room for a *description* of the coding style that
happens to be currently in use.  Especially if it comes with sufficient
caveats to clarify that it's not a rule book.

E.g. it could have a few rules, together with a percentage of how often
it is followed.  Could be quite useful for ethnography and maybe also
comparative "literature", or to classify Free Software projects based on
their preferred coding styles.


        Stefan

Re: Codifying some aspects of Elisp code style and improving pretty printer

[Robert Pluim]

>>>>> On Fri, 01 Oct 2021 09:05:25 +0200, Lars Ingebrigtsen <larsi@gnus.org> said:

    Lars> akater <nuclearspace@gmail.com> writes:
    >> But I don't think in Lisp this could be enforced in its entirety which
    >> is why I said that indentation mechanism should not attempt to enforce
    >> this.  It's very subjective.  Often, I want to keep a long line and a
    >> couple of moments afterwards I decide that it's too long after all.

    Lars> A coding style that's not automatically enforced isn't very useful, in
    Lars> my opinion.  It just leads to even more arguing, with some people
    Lars> insisting that it should always be adhered to, and others arguing that
    Lars> this is a special case where it shouldn't.

Apropos, I believe the current recommendation is to do

    (mapconcat #'identity seqs " ")

rather than

    (mapconcat 'identity seqs " ")

ie function-quoting the argument to mapconcat, apply, mapc etc, but

1. The elisp manual doesnʼt say that in (info "(elisp) Mapping Functions")
2. Emacs' code itself doesnʼt do that consistently (and we have the
odd #'(lambda ...) as well
3. The byte compiler doesnʼt warn about it.

I have a byte-compiler hack for [3] that could be used to fix [2], and
[1] is easily changed. But do we want to? I believe it helps the
byte-compiler generate better code, but donʼt know what the actual
effect is.

    Lars> We (the maintainers) usually touch up code before committing to adhere
    Lars> to our opinions of how the code should look, and hope that submitters
    Lars> pick up on it after a while (so that we don't have to keep on doing it).

Yes, but sometimes 'run your code through this tool first' encourages
people to conform, and saves you time.

Robert
-- 

Re: Codifying some aspects of Elisp code style and improving pretty printer

[Lars Ingebrigtsen]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

> I guess there is room for a *description* of the coding style that
> happens to be currently in use.  Especially if it comes with sufficient
> caveats to clarify that it's not a rule book.

Yeah, sure -- a section of "things we think are rather spiffy when
formatting Emacs Lisp" would be nice.  With lots of examples.

> E.g. it could have a few rules, together with a percentage of how often
> it is followed.  Could be quite useful for ethnography and maybe also
> comparative "literature", or to classify Free Software projects based on
> their preferred coding styles.

Heh heh heh.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Re: Codifying some aspects of Elisp code style and improving pretty printer

[Stefan Monnier]

> Apropos, I believe the current recommendation is to do
>     (mapconcat #'identity seqs " ")
> rather than
>     (mapconcat 'identity seqs " ")
> ie function-quoting the argument to mapconcat, apply, mapc etc, but

Indeed, the main benefit being that the byte-compiler can warn you if
the function is not known to be defined (typically because of a typo or
a missing `require`).

A side benefit is that it "does the right thing" for `cl-flet` and friends.

> 1. The elisp manual doesnʼt say that in (info "(elisp) Mapping Functions")

Not sure that'd be the best place, but we should probably adjust the
manual to encourage the use of #' to quote function names,

> 2. Emacs' code itself doesnʼt do that consistently

The difference is sufficiently unimportant that I don't see a need to
try and make it consistent in this respect.  E.g. I do change 'foo to
#'foo quite often, but only as part of other changes.

> (and we have the odd #'(lambda ...) as well

I don't see anything wrong with #'(lambda...)
[ I personally prefer to skip the #' to make the code (marginally)
  shorter, but it's purely a matter of taste and I usually try and
  refrain from making this change in existing code unless it helps make
  the code fit into 80 columns.  ]

> 3. The byte compiler doesnʼt warn about it.

Mine does ;-)
But I wouldn't want to enable such a warning by default (or at least it
should be mess a bit less eager than it is, because of the occasional false
positives).

> I believe it helps the byte-compiler generate better code, but donʼt
> know what the actual effect is.

It has no effect on the generated code.


        Stefan

Re: Codifying some aspects of Elisp code style and improving pretty printer

[Robert Pluim]

>>>>> On Fri, 01 Oct 2021 11:26:51 -0400, Stefan Monnier <monnier@iro.umontreal.ca> said:

    >> Apropos, I believe the current recommendation is to do
    >> (mapconcat #'identity seqs " ")
    >> rather than
    >> (mapconcat 'identity seqs " ")
    >> ie function-quoting the argument to mapconcat, apply, mapc etc, but

    Stefan> Indeed, the main benefit being that the byte-compiler can warn you if
    Stefan> the function is not known to be defined (typically because of a typo or
    Stefan> a missing `require`).

    Stefan> A side benefit is that it "does the right thing" for `cl-flet` and friends.

    >> 1. The elisp manual doesnʼt say that in (info "(elisp) Mapping Functions")

    Stefan> Not sure that'd be the best place, but we should probably adjust the
    Stefan> manual to encourage the use of #' to quote function names,

All the examples there use 'quote', and the discussion of 'quote' vs
'function' is in (info "(elisp) Anonymous Functions"). People who've
just gone 'i mapc' in the elisp manual have no reason to read that
second node. Patch below

    >> 2. Emacs' code itself doesnʼt do that consistently

    Stefan> The difference is sufficiently unimportant that I don't see a need to
    Stefan> try and make it consistent in this respect.  E.g. I do change 'foo to
    Stefan> #'foo quite often, but only as part of other changes.

OK. Thatʼs ~900 spurious changes we can avoid making :-)

    >> (and we have the odd #'(lambda ...) as well

    Stefan> I don't see anything wrong with #'(lambda...)
    Stefan> [ I personally prefer to skip the #' to make the code (marginally)
    Stefan>   shorter, but it's purely a matter of taste and I usually try and
    Stefan>   refrain from making this change in existing code unless it helps make
    Stefan>   the code fit into 80 columns.  ]

If dislike it, because I view lambda as self-quoting, but as you say
thatʼs a personal preference (although "(elisp) Anonymous Functions"
kind of tells you not to quote lambda's).

    >> 3. The byte compiler doesnʼt warn about it.

    Stefan> Mine does ;-)
    Stefan> But I wouldn't want to enable such a warning by default (or at least it
    Stefan> should be mess a bit less eager than it is, because of the occasional false
    Stefan> positives).

Iʼve not seen any false positives from mine, but I suspect itʼs less
sophisticated than yours :-)

    >> I believe it helps the byte-compiler generate better code, but donʼt
    >> know what the actual effect is.

    Stefan> It has no effect on the generated code.

OK, so no code change, but how about:

diff --git a/doc/lispref/functions.texi b/doc/lispref/functions.texi
index c856557c3c..4416b39f09 100644
--- a/doc/lispref/functions.texi
+++ b/doc/lispref/functions.texi
@@ -890,6 +890,33 @@ Mapping Functions
 over a char-table in a way that deals properly with its sparse nature,
 use the function @code{map-char-table} (@pxref{Char-Tables}).
 
+  All the mapping functions described here take a function as their
+first argument, which can be specified in different ways:
+
+@itemize
+@item An anonymous lambda
+
+@example
+(mapcar (lambda (x) (x)) ....)
+@end example
+
+@item A function-quoted symbol
+
+@example
+(mapcar #'identity ....)
+@end example
+
+@item An quoted symbol
+
+@example
+(mapcar 'car ....)
+@end example
+@end itemize
+
+The last two are equivalent, except that function-quoting can help the
+byte-compiler warn if the function being called is
+undefined.  @xref{Anonymous Functions} for more information.
+
 @defun mapcar function sequence
 @anchor{Definition of mapcar}
 @code{mapcar} applies @var{function} to each element of @var{sequence}

Re: Codifying some aspects of Elisp code style and improving pretty printer

[Stefan Monnier]

> All the examples there use 'quote', and the discussion of 'quote' vs
> 'function' is in (info "(elisp) Anonymous Functions"). People who've
> just gone 'i mapc' in the elisp manual have no reason to read that
> second node. Patch below

See comment below.

> If dislike it, because I view lambda as self-quoting, but as you say
> thatʼs a personal preference (although "(elisp) Anonymous Functions"
> kind of tells you not to quote lambda's).

Quoting a lambda for me means to put a `quote` in front of it not
a `function`.  #' is a "translucent quote" which says this isn't some
chunk of data (something the compiler basically can't touch nor
"understand") but is a chunk of code (something which the compiler can
definitely manipulate, analyze, etc...).

>     >> 3. The byte compiler doesnʼt warn about it.
>
>     Stefan> Mine does ;-)
>     Stefan> But I wouldn't want to enable such a warning by default (or at least it
>     Stefan> should be mess a bit less eager than it is, because of the occasional false
>     Stefan> positives).
>
> Iʼve not seen any false positives from mine, but I suspect itʼs less
> sophisticated than yours :-)

Mine's very simple, but I think most of its false positives come from
`define-key`. In any case I think the trade off between annoying coders
with more warnings and the improvement in code quality we hope to get by
fixing the code to eliminate the warnings is not in favor of adding
this warning.  At least not at this point.

> diff --git a/doc/lispref/functions.texi b/doc/lispref/functions.texi
> index c856557c3c..4416b39f09 100644
> --- a/doc/lispref/functions.texi
> +++ b/doc/lispref/functions.texi
> @@ -890,6 +890,33 @@ Mapping Functions
>  over a char-table in a way that deals properly with its sparse nature,
>  use the function @code{map-char-table} (@pxref{Char-Tables}).
>  
> +  All the mapping functions described here take a function as their
> +first argument, which can be specified in different ways:
> +
> +@itemize
> +@item An anonymous lambda
> +
> +@example
> +(mapcar (lambda (x) (x)) ....)
> +@end example
> +
> +@item A function-quoted symbol
> +
> +@example
> +(mapcar #'identity ....)
> +@end example
> +
> +@item An quoted symbol
> +
> +@example
> +(mapcar 'car ....)
> +@end example
> +@end itemize

I don't see the point of documenting (mapcar 'foo ..) at this place.
Also functions can be specified in many other ways (they can come from
variable or be computed on the spot by calling a function-generating
function, such as `apply-partially`).

So I think it's better to link to some other place in the manual where
we talk specifically about anonymous functions.

More importantly, we should try and systematically use #'foo in all the
examples instead.

Also I wouldn't call it "a (function-)quoted symbol" but I'd say that we
pass to mapcar a "function name" or maybe a "named function".  We can
elsewhere mention that 'foo also works to pass a (global)
named function.


        Stefan