bug#2160: 23.0.60; Indentation of LOOP (cl-indent.el)

bug#2160: 23.0.60; Indentation of LOOP (cl-indent.el)

[tcr]

When using `common-lisp-indent-function', LOOP forms are by default
indented as follows

  (loop for x in '(a b c)
     for y in '(1 2 3)
     collect (cons x y))

Setting the following

  (setq lisp-simple-loop-indentation 1
        lisp-loop-keyword-indentation 6
        lisp-loop-forms-indentation 6)

makes it be indented like

  (loop for x in '(a b c)
        for y in '(1 2 3)
        collect (cons x y))

which I think is more readable, and more canonical.

I hence propose to change the default values in cl-indent.el of the
above variables accordingly.

  -T.


In GNU Emacs 23.0.60.1 (i486-pc-linux-gnu, GTK+ Version 2.12.9)
 of 2008-03-19 on vernadsky, modified by Debian
 (emacs-snapshot package, version 1:20080228-1ubuntu1)
Windowing system distributor `The X.Org Foundation', version 11.0.10400090
configured using `configure  '--build' 'i486-linux-gnu' '--host' 'i486-linux-gnu' '--prefix=/usr' '--sharedstatedir=/var/lib' '--libexecdir=/usr/lib' '--localstatedir=/var' '--infodir=/usr/share/info' '--mandir=/usr/share/man' '--with-pop=yes' '--enable-locallisppath=/etc/emacs-snapshot:/etc/emacs:/usr/local/share/emacs/23.0.60/site-lisp:/usr/local/share/emacs/site-lisp:/usr/share/emacs/23.0.60/site-lisp:/usr/share/emacs/site-lisp:/usr/share/emacs/23.0.60/leim' '--with-x=yes' '--with-x-toolkit=gtk' '--enable-font-backend' 'build_alias=i486-linux-gnu' 'host_alias=i486-linux-gnu' 'CFLAGS=-DDEBIAN -DSITELOAD_PURESIZE_EXTRA=5000 -g -O2' 'LDFLAGS=-g -Wl,--as-needed' 'CPPFLAGS=''

Important settings:
  value of $LC_ALL: nil
  value of $LC_COLLATE: nil
  value of $LC_CTYPE: nil
  value of $LC_MESSAGES: nil
  value of $LC_MONETARY: nil
  value of $LC_NUMERIC: nil
  value of $LC_TIME: nil
  value of $LANG: en_US.UTF-8
  value of $XMODIFIERS: nil
  locale-coding-system: utf-8-unix
  default-enable-multibyte-characters: t

Major mode: Group

Minor modes in effect:
  gnus-topic-mode: t
  gnus-undo-mode: t
  shell-dirtrack-mode: t
  tooltip-mode: t
  mouse-wheel-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  global-auto-composition-mode: t
  auto-composition-mode: t
  auto-compression-mode: t
  column-number-mode: t
  line-number-mode: t
  transient-mark-mode: t

-- 
Diese Nachricht wurde auf Viren und andere gefaerliche Inhalte untersucht
und ist - aktuelle Virenscanner vorausgesetzt - sauber.
Freebits E-Mail Virus Scanner

bug#2160: 23.0.60; Indentation of LOOP (cl-indent.el)

[Lars Ingebrigtsen]

tcr@freebits.de writes:

> When using `common-lisp-indent-function', LOOP forms are by default
> indented as follows
>
>   (loop for x in '(a b c)
>      for y in '(1 2 3)
>      collect (cons x y))

(I'm going through old bug reports that unfortunately didn't get any
response at the time.)

This is still the case in Emacs 27.

> Setting the following
>
>   (setq lisp-simple-loop-indentation 1
>         lisp-loop-keyword-indentation 6
>         lisp-loop-forms-indentation 6)
>
> makes it be indented like
>
>   (loop for x in '(a b c)
>         for y in '(1 2 3)
>         collect (cons x y))
>
> which I think is more readable, and more canonical.
>
> I hence propose to change the default values in cl-indent.el of the
> above variables accordingly.

I agree that changing the default here would be a good idea, because
that's a very non-standard Common Lisp indentation.  Does anybody object
to changing it?

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

bug#2160: 23.0.60; Indentation of LOOP (cl-indent.el)

[Stefan Kangas]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> tcr@freebits.de writes:
>
>> When using `common-lisp-indent-function', LOOP forms are by default
>> indented as follows
>>
>>   (loop for x in '(a b c)
>>      for y in '(1 2 3)
>>      collect (cons x y))
>
> (I'm going through old bug reports that unfortunately didn't get any
> response at the time.)
>
> This is still the case in Emacs 27.
>
>> Setting the following
>>
>>   (setq lisp-simple-loop-indentation 1
>>         lisp-loop-keyword-indentation 6
>>         lisp-loop-forms-indentation 6)
>>
>> makes it be indented like
>>
>>   (loop for x in '(a b c)
>>         for y in '(1 2 3)
>>         collect (cons x y))
>>
>> which I think is more readable, and more canonical.
>>
>> I hence propose to change the default values in cl-indent.el of the
>> above variables accordingly.
>
> I agree that changing the default here would be a good idea, because
> that's a very non-standard Common Lisp indentation.  Does anybody object
> to changing it?

No further comments within 21 weeks, so I'll just chime in to say the
suggestion makes sense to me.  It seems to be the usual indentation in
the Common Lisp world, and I see no reason to do things differently.
It certainly makes the code more readable.

People have had ample time to protest, so perhaps we should just go
ahead and make the change on the master branch.

Best regards,
Stefan Kangas

bug#2160: 23.0.60; Indentation of LOOP (cl-indent.el)

[Lars Ingebrigtsen]

Stefan Kangas <stefan@marxist.se> writes:

> People have had ample time to protest, so perhaps we should just go
> ahead and make the change on the master branch.

Yup.  

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

bug#2160: 23.0.60; Indentation of LOOP (cl-indent.el)

[Stefan Kangas]

Lars Ingebrigtsen <larsi@gnus.org> writes:

>> People have had ample time to protest, so perhaps we should just go
>> ahead and make the change on the master branch.
>
> Yup.  

I just realized that the suggestion as is makes code like this indent
weirdly:

    (loop
          for item in list
          for i from 1 to 10
          do (something))

The better indenting in my taste would be what we have now:

    (loop
       for item in list
       for i from 1 to 10
       do (something))

This is from an example I found online.  Not sure how common this is
in practice.

Best regards,
Stefan Kangas

bug#2160: 23.0.60; Indentation of LOOP (cl-indent.el)

[Lars Ingebrigtsen]

Stefan Kangas <stefan@marxist.se> writes:

> I just realized that the suggestion as is makes code like this indent
> weirdly:
>
>     (loop
>           for item in list
>           for i from 1 to 10
>           do (something))
>
> The better indenting in my taste would be what we have now:
>
>     (loop
>        for item in list
>        for i from 1 to 10
>        do (something))
>
> This is from an example I found online.  Not sure how common this is
> in practice.

The former looks like what I'd expect (as a Common Lisp programmer).
Except that I would never ever do a newline after "(loop ".

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

bug#2160: 23.0.60; Indentation of LOOP (cl-indent.el)

[Stefan Kangas]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> Stefan Kangas <stefan@marxist.se> writes:
>
>> I just realized that the suggestion as is makes code like this indent
>> weirdly:
>>
>>     (loop
>>           for item in list
>>           for i from 1 to 10
>>           do (something))
>>
>> The better indenting in my taste would be what we have now:
>>
>>     (loop
>>        for item in list
>>        for i from 1 to 10
>>        do (something))
>>
>> This is from an example I found online.  Not sure how common this is
>> in practice.
>
> The former looks like what I'd expect (as a Common Lisp programmer).
> Except that I would never ever do a newline after "(loop ".

OK, thanks.  Let's push to master then.

Best regards,
Stefan Kangas

bug#2160: 23.0.60; Indentation of LOOP (cl-indent.el)

[Stefan Kangas]

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

Stefan Kangas <stefan@marxist.se> writes:

> Lars Ingebrigtsen <larsi@gnus.org> writes:
>
>> The former looks like what I'd expect (as a Common Lisp programmer).
>> Except that I would never ever do a newline after "(loop ".
>
> OK, thanks.  Let's push to master then.

Please find attached the patch I suggest.  I'll allow a couple of days
before I push it in case anyone has any comments.

Best regards,
Stefan Kangas


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Improve-indentation-of-loop-forms.patch --]
[-- Type: text/x-diff, Size: 2787 bytes --]

From e8d3cb1601e9b83e743090f493befae103745fc9 Mon Sep 17 00:00:00 2001
From: Stefan Kangas <stefankangas@gmail.com>
Date: Thu, 30 Apr 2020 18:11:28 +0200
Subject: [PATCH] Improve indentation of 'loop' forms

* lisp/emacs-lisp/cl-indent.el (lisp-loop-keyword-indentation)
(lisp-loop-forms-indentation, lisp-simple-loop-indentation): Use a
more standard indentation of 'loop' forms.  (Bug#2160)

(lisp-indent-maximum-backtracking, lisp-tag-indentation)
(lisp-tag-body-indentation, lisp-backquote-indentation)
(lisp-loop-keyword-indentation, lisp-loop-forms-indentation)
(lisp-simple-loop-indentation): Remove redundant :group args.
---
 lisp/emacs-lisp/cl-indent.el | 24 ++++++++++--------------
 1 file changed, 10 insertions(+), 14 deletions(-)

diff --git a/lisp/emacs-lisp/cl-indent.el b/lisp/emacs-lisp/cl-indent.el
index fd8715962a..361414ef1f 100644
--- a/lisp/emacs-lisp/cl-indent.el
+++ b/lisp/emacs-lisp/cl-indent.el
@@ -46,14 +46,12 @@ lisp-indent-maximum-backtracking
   "Maximum depth to backtrack out from a sublist for structured indentation.
 If this variable is 0, no backtracking will occur and forms such as `flet'
 may not be correctly indented."
-  :type 'integer
-  :group 'lisp-indent)
+  :type 'integer)
 
 (defcustom lisp-tag-indentation 1
   "Indentation of tags relative to containing list.
 This variable is used by the function `lisp-indent-tagbody'."
-  :type 'integer
-  :group 'lisp-indent)
+  :type 'integer)
 
 (defcustom lisp-tag-body-indentation 3
   "Indentation of non-tagged lines relative to containing list.
@@ -64,32 +62,30 @@ lisp-tag-body-indentation
 as a block at the same indentation as the first s-expression following
 the tag.  In this case, any forms before the first tag are indented
 by `lisp-body-indent'."
-  :type 'integer
-  :group 'lisp-indent)
+  :type 'integer)
 
 (defcustom lisp-backquote-indentation t
   "Whether or not to indent backquoted lists as code.
 If nil, indent backquoted lists as data, i.e., like quoted lists."
-  :type 'boolean
-  :group 'lisp-indent)
+  :type 'boolean)
 
 
-(defcustom lisp-loop-keyword-indentation 3
+(defcustom lisp-loop-keyword-indentation 6
   "Indentation of loop keywords in extended loop forms."
   :type 'integer
-  :group 'lisp-indent)
+  :version "28.1")
 
 
-(defcustom lisp-loop-forms-indentation 5
+(defcustom lisp-loop-forms-indentation 6
   "Indentation of forms in extended loop forms."
   :type 'integer
-  :group 'lisp-indent)
+  :version "28.1")
 
 
-(defcustom lisp-simple-loop-indentation 3
+(defcustom lisp-simple-loop-indentation 1
   "Indentation of forms in simple loop forms."
   :type 'integer
-  :group 'lisp-indent)
+  :version "28.1")
 
 (defcustom lisp-lambda-list-keyword-alignment nil
   "Whether to vertically align lambda-list keywords together.
-- 
2.26.2

bug#2160: 23.0.60; Indentation of LOOP (cl-indent.el)

[Stefan Kangas]

close 2160 28.1
thanks

Stefan Kangas <stefan@marxist.se> writes:

> Please find attached the patch I suggest.  I'll allow a couple of days
> before I push it in case anyone has any comments.

Pushed to master as commit d55b3b5966.  Closing.

Best regards,
Stefan Kangas