Common Lisp indentation bug fix/new feature

Common Lisp indentation bug fix/new feature

[Lars Magne Ingebrigtsen]

This is how much (most? some? I haven't actually counted) Common Lisp
code found out in the world looks like:

(defun foo ()
  (loop for z from 1 upto 5
        do (princ z)
           (terpri)
           (terpri)))

That is, the statements after DO/DOING/FINALLY/etc line up so that you
can see that they are part of the same block of code.

Unfortunately, the CL indentation code does this instead:

  (loop for z from 1 upto 5
        do (princ z)
          (terpri)
          (terpri)

Now, one can adjust the indentation with `lisp-loop-forms-indentation',
so I could just increase it by 1 and things would be OK?  Nope, since 
then FINALLY would look like:

  (loop for z from 1 upto 5
        finally (princ z)
           (terpri)
           (terpri)

With the following patch (and a defcustom adjustment), we would allow
getting the desired behaviour.

The defaults don't change, though, so this wouldn't be a user-visible
change, unless the set `lisp-loop-forms-indentation' to nil.

So would it be OK to apply this, or is it too new-featurish?  I kinda
think it fixes an indentation bug...

=== modified file 'lisp/emacs-lisp/cl-indent.el'
--- lisp/emacs-lisp/cl-indent.el	2011-11-21 21:58:38 +0000
+++ lisp/emacs-lisp/cl-indent.el	2011-11-21 22:15:28 +0000
@@ -152,11 +152,26 @@
     (error t)))
 
 
+(defun common-lisp-loop-keyword-length (loop-start)
+  "Return the length of the preceding loop keyword.
+Stop looking before LOOP-START."
+  (save-excursion
+    (let ((length 0))
+      (while (and (zerop length)
+		  (> (point) loop-start))
+	(beginning-of-line)
+	(when (looking-at "^\\s-*\\(loop\\s-*\\)?\\(:?\\sw+\\|;\\)")
+	  (setq length (length (match-string 2))))
+	(forward-line -1))
+      length)))
+
+
 (defun common-lisp-loop-part-indentation (indent-point state)
   "Compute the indentation of loop form constituents."
-  (let* ((loop-indentation (save-excursion
-			     (goto-char (elt state 1))
-			     (current-column))))
+  (let ((loop-indentation (save-excursion
+			    (goto-char (elt state 1))
+			    (current-column)))
+	(case-fold-search t))
     (goto-char indent-point)
     (beginning-of-line)
     (list
@@ -165,7 +180,13 @@
 	   ((looking-at "^\\s-*\\(:?\\sw+\\|;\\)")
 	    (+ loop-indentation lisp-loop-keyword-indentation))
 	   (t
-	    (+ loop-indentation lisp-loop-forms-indentation)))
+	    (+ loop-indentation
+	       lisp-loop-keyword-indentation
+	       (or lisp-loop-forms-indentation
+		   (1+ (common-lisp-loop-keyword-length
+			(or (save-excursion
+			      (re-search-backward "(\\s-*loop" nil t))
+			    indent-point)))))))
      ;; Tell the caller that the next line needs recomputation, even
      ;; though it doesn't start a sexp.
      loop-indentation)))



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

Re: Common Lisp indentation bug fix/new feature

[Stefan Monnier]

> So would it be OK to apply this, or is it too new-featurish?  I kinda
> think it fixes an indentation bug...

I think it's a good change, but I don't think there's any rush to put it
into Emacs-24.1.


        Stefan

Re: Common Lisp indentation bug fix/new feature

[Lars Magne Ingebrigtsen]

Stefan Monnier <monnier@IRO.UMontreal.CA> writes:

> I think it's a good change, but I don't think there's any rush to put it
> into Emacs-24.1.

Right.  And it's not a totally complete solution, either.  Stuff like

(loop for (bar foo)
      on zot
      by #'cddr
      collect bar)

should probably not indent like that?  (Which is what it does both now
and with my additional patch, I think.)

I think the LOOP indentation thing really needs to have deeper knowledge
about the semantics to indent stuff better.  That is, it needs to know
that FOR and COLLECT introduce a new clause, while ON/BY don't.

So I'll write something more semantically aware and apply it to Emacs
after 24.1.

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

Re: Common Lisp indentation bug fix/new feature

[Lawrence Mitchell]

Lars Magne Ingebrigtsen wrote:
> Stefan Monnier <monnier@IRO.UMontreal.CA> writes:

>> I think it's a good change, but I don't think there's any rush to put it
>> into Emacs-24.1.

> Right.  And it's not a totally complete solution, either.  Stuff like

> (loop for (bar foo)
>       on zot
>       by #'cddr
>       collect bar)

> should probably not indent like that?  (Which is what it does both now
> and with my additional patch, I think.)

> I think the LOOP indentation thing really needs to have deeper knowledge
> about the semantics to indent stuff better.  That is, it needs to know
> that FOR and COLLECT introduce a new clause, while ON/BY don't.

> So I'll write something more semantically aware and apply it to Emacs
> after 24.1.

Are you aware of the existing efforts in this area, namely the
slime-cl-indent that's being developed in SLIME.  See
http://www.didierverna.com/sciblog/index.php?post/2011/05/06/Common-Lisp-indentation-in-XEmacs
and http://random-state.net/log/3513839648.html

Perhaps it could be synced into Emacs.

Lawrence
-- 
Lawrence Mitchell <wence@gmx.li>

Re: Common Lisp indentation bug fix/new feature

[Stefan Monnier]

> I think the LOOP indentation thing really needs to have deeper knowledge
> about the semantics to indent stuff better.  That is, it needs to know
> that FOR and COLLECT introduce a new clause, while ON/BY don't.
> So I'll write something more semantically aware and apply it to Emacs
> after 24.1.

I'd be interested to see how SMIE handles it.
I.e. something along the lines of

  (defvar lisp-loop-smie-grammar
    (smie-prec2->grammar
     (smie-bnf->prec2 BLABLA)))

  ...whileindenting...
  ...(if (head-is-loop)
        (let ((smie-grammar lisp-loop-smie-grammar)
              (smie-rules-function BLEBLE)
              (smie-forward-token-function BLIBLI)
              (smie-backward-token-function BLOBLO))
          (smie-indent-calculate)))


        Stefan

Re: Common Lisp indentation bug fix/new feature

[Lars Magne Ingebrigtsen]

Stefan Monnier <monnier@IRO.UMontreal.CA> writes:

> I'd be interested to see how SMIE handles it.

I don't know from SMIE, but I was told by my cow-orkers today that SLIME
has its own version of cl-indent.el that does LOOP indentation
properly.  (Allegedly the only difference between the version in SLIME
and Emacs is the LOOP handling.)

It indents like this:

  (loop for z
	  from 1 upto 5
	do (princ z)
	   (terpri)
	   (terpri)
	when (zerop z)
	  finally (terpri)
		  (terpri))

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

Re: Common Lisp indentation bug fix/new feature

[Tim Cross]

The LOOP macro in CL is quite a complex/powerful beastie - a language
in/of itself really. Itcertainly has special indentation/formatting
requirements. I guess this has never really been addressed because
there are very few people who really know and understand the power of
LOOP. Most I've met know specific parts quite well, but often express
surprise when someone else points out alternative ways of expressing
similar/same constructs  etc.

I would suggest that taking a cue from the SLIME developers is
probably the wisest route to follow as I suspect they are probably the
largest user group for CL coding under emacs. Regardless of how
good/bad the native/standard mode provided by emacs is, if the way it
does indenting is not generally accepted by the SLIME community, any
efforts put in by emacs devs is largely wasted.  I would expect that
any proposals concerning LOOP in CL will likely generate some debate -
it is one of those constructs which seems to generate a fair amount of
emotional responses - either you like it or hate it and those who like
it seem to like it for very different reasons and have quite strong
views on how it should be used and formatted.

Tim

On Wed, Nov 23, 2011 at 5:09 AM, Lars Magne Ingebrigtsen <larsi@gnus.org> wrote:
> Stefan Monnier <monnier@IRO.UMontreal.CA> writes:
>
>> I'd be interested to see how SMIE handles it.
>
> I don't know from SMIE, but I was told by my cow-orkers today that SLIME
> has its own version of cl-indent.el that does LOOP indentation
> properly.  (Allegedly the only difference between the version in SLIME
> and Emacs is the LOOP handling.)
>
> It indents like this:
>
>  (loop for z
>          from 1 upto 5
>        do (princ z)
>           (terpri)
>           (terpri)
>        when (zerop z)
>          finally (terpri)
>                  (terpri))
>
> --
> (domestic pets only, the antidote for overdose, milk.)
>  bloggy blog http://lars.ingebrigtsen.no/
>
>



-- 
Tim Cross

Re: Common Lisp indentation bug fix/new feature

[Nikodemus Siivola]

On 22 November 2011 20:09, Lars Magne Ingebrigtsen <larsi@gnus.org> wrote:

> I don't know from SMIE, but I was told by my cow-orkers today that SLIME
> has its own version of cl-indent.el that does LOOP indentation
> properly.  (Allegedly the only difference between the version in SLIME
> and Emacs is the LOOP handling.)

Missed this when it came up, but FWIW, plenty more differences have
crept in since then as I realized that cl-indent.el in Emacs was
sitting still and I stopped worried about staying in synch.

Partial list:

* Test suite!

* Support for named indentation styles.

* Better DEFGENERIC and DEFMETHOD indentation.

* Better lambda-list indentation.

* LOOP indentation, as mentioned.

* Aligning keywords in calls.

* Better handling of comments. There is still plenty of work to be
done here, though. Comment indentation remains my biggest irritant,
but at least we now get

        (foo ;; Deal with bar
             (bar)
             ;; Deal with quux
             (quux))

instead of

        (foo ;; Deal with bar
         (bar)
         ;; Deal with quux
         (quux))

* CL feature expression handling. It's not perfect, but a damn sight
better than nothing.

* Better DEFCLASS and DEFINE-CONDITION superclass list indentation.

* Fixed handling of incomplete destructuring indentation specs.

* Don't consider DEFAULT, DEFINER and DEFINITION to be defun-like.

* Indent boa-constructor lambda-lists properly.

* Fix handling of , and ,@ at the start of the indentation.

* Indirect indentation specs. (late bound)

* Fallback method for trailing expressions on prev line: Emacs'
calculate-lisp-indent doesn't indent

        (foo (or x
                 y) t
             z)

right, but would align Z with Y.

* Support for IF*.

Cheers,

 -- nikodemus

Re: Common Lisp indentation bug fix/new feature

[Lars Magne Ingebrigtsen]

Nikodemus Siivola <nikodemus@random-state.net> writes:

> Missed this when it came up, but FWIW, plenty more differences have
> crept in since then as I realized that cl-indent.el in Emacs was
> sitting still and I stopped worried about staying in synch.

Perhaps we could merge the cl-indent.el changes to Emacs after Emacs
24.1 has been released?

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

Re: Common Lisp indentation bug fix/new feature

[Nikodemus Siivola]

On 3 April 2012 06:47, Lars Magne Ingebrigtsen <larsi@gnus.org> wrote:
> Nikodemus Siivola <nikodemus@random-state.net> writes:
>
>> Missed this when it came up, but FWIW, plenty more differences have
>> crept in since then as I realized that cl-indent.el in Emacs was
>> sitting still and I stopped worried about staying in synch.
>
> Perhaps we could merge the cl-indent.el changes to Emacs after Emacs
> 24.1 has been released?

I'm not opposed, but there's a bit of legwork to chase FSF copyright
release forms required: with the loop indentation code in particular
it can be tricky to find the original people involved.

That said, except for the subclause-aware loop indentation everyone
should be easy to reach -- and that part should be easy to rip out. I
can factor it out into a separate file in the Slime repo at some
point.

Cheers,

 -- nikodemus

Re: Common Lisp indentation bug fix/new feature

[Stefan Monnier]

> That said, except for the subclause-aware loop indentation everyone
> should be easy to reach -- and that part should be easy to rip out.
> I can factor it out into a separate file in the Slime repo at
> some point.

If you could do that and get the list of people involved that would
be great.  We can take care of contacting them to do the paperwork (for
those who haven't done it yet).


        Stefan