bug#16202: 24.3.50; lisp-mode's indent-sexp might be buggy (for slime in particular, and lisp in general)

bug#16202: 24.3.50; lisp-mode's indent-sexp might be buggy (for slime in particular, and lisp in general)

[João Távora]

Hi maintainers,

This was started by bug reports in the slime-devel mailing list

  http://comments.gmane.org/gmane.lisp.slime.devel/11196
  http://comments.gmane.org/gmane.lisp.slime.devel/11189

The reproduction recipe described in these messages requires,
unfortunately, that you install slime.

However, I do think there is a slight bug on emacs's side (which is
probably not visible in emacs-lisp's testing rules). You see, the common
lisp loop macro likes its comments indented like (this is slime's test
nr 11).

   (loop when foo
           do (fubar)
           and collect cash
         else do ;; this is the body of the first else
                 ;; the body is ...
                 (indented to the above comment)
                 (ZMACS gets this wrong)
         do
         when funny-predicate do ;; Here's a comment
                                 (body filled to comment))

and sometimes indent-sexp insists on

   (loop when foo
           do (fubar)
           and collect cash
         else do       ;; this is the body of the first else
                       ;; the body is ...
                 (indented to the above comment)
                 (ZMACS gets this wrong)
         do
         when funny-predicate do ;; Here's a comment
                                 (body filled to comment))

which indented the two comment lines to comment-column, I think. The
patch I attach at the end of this message fixes it.

...as does indenting the sexp with indent-region. This might be naive,
but why not super-simplify indent-sexp to be something like this?

   (defun indent-sexp (&optional endpos)
     (interactive)
     (if endpos
         (indent-region (point) endpos)
       (indent-region (point) (save-excursion
                                (forward-sexp 1)
                                (point)))))

Thanks,
João

*** d:/joaot/Vendor/emacs-24.3.5-old/share/emacs/24.3.50/lisp/emacs-lisp/lisp-mode.el
--- #<buffer lisp-mode.el|emacs-24.3.5-old>
***************
*** 1578,1596 ****
  	;; unless a line ends inside a string.
  	(while (and (not inner-loop-done)
  		    (not (setq outer-loop-done (eobp))))
  	  (setq state (parse-partial-sexp (point) (progn (end-of-line) (point))
  					  nil nil state))
  	  (setq next-depth (car state))
! 	  ;; If the line contains a comment other than the sort
! 	  ;; that is indented like code,
! 	  ;; indent it now with indent-for-comment.
! 	  ;; Comments indented like code are right already.
! 	  ;; In any case clear the in-comment flag in the state
! 	  ;; because parse-partial-sexp never sees the newlines.
! 	  (if (car (nthcdr 4 state))
! 	      (progn (indent-for-comment)
! 		     (end-of-line)
! 		     (setcar (nthcdr 4 state) nil)))
  	  ;; If this line ends inside a string,
  	  ;; go straight to next line, remaining within the inner loop,
  	  ;; and turn off the \-flag.
--- 1578,1597 ----
  	;; unless a line ends inside a string.
  	(while (and (not inner-loop-done)
  		    (not (setq outer-loop-done (eobp))))
+ 	  (setq before-parse (point))
  	  (setq state (parse-partial-sexp (point) (progn (end-of-line) (point))
  					  nil nil state))
  	  (setq next-depth (car state))
! 	  ;; If the line contains nothing but a comment other than the sort that
! 	  ;; is indented like code, indent it now with indent-for-comment.
! 	  ;; Comments indented like code are right already.  In any case clear
! 	  ;; the in-comment flag in the state because parse-partial-sexp never
! 	  ;; sees the newlines.
! 	  (when (car (nthcdr 4 state))
!             (when (<= (nth 8 state) before-parse)
!               (indent-for-comment)
!               (end-of-line))
!             (setcar (nthcdr 4 state) nil))
  	  ;; If this line ends inside a string,
  	  ;; go straight to next line, remaining within the inner loop,
  	  ;; and turn off the \-flag.

Diff finished.  Fri Dec 20 13:30:32 2013

bug#16202: 24.3.50; lisp-mode's indent-sexp might be buggy (for slime in particular, and lisp in general)

[npostavs]

joaotavora@gmail.com (João Távora) writes:

> ...as does indenting the sexp with indent-region. This might be naive,
> but why not super-simplify indent-sexp to be something like this?
>
>    (defun indent-sexp (&optional endpos)
>      (interactive)
>      (if endpos
>          (indent-region (point) endpos)
>        (indent-region (point) (save-excursion
>                                 (forward-sexp 1)
>                                 (point)))))

Just for the record, at least one significant difference is that
indent-sexp also indents comments at the end of the line, while
indent-region does not.

e.g.

(progn
       ;; comment on its own line
  (do-something) ; a comment which is not moved by indent-region
  (do-another-thing))

bug#16202: 24.3.50; lisp-mode's indent-sexp might be buggy (for slime in particular, and lisp in general)

[npostavs]

tags 16202 notabug
quit

joaotavora@gmail.com (João Távora) writes:

> Hi maintainers,
>
> This was started by bug reports in the slime-devel mailing list
>
>   http://comments.gmane.org/gmane.lisp.slime.devel/11196
>   http://comments.gmane.org/gmane.lisp.slime.devel/11189
>
> The reproduction recipe described in these messages requires,
> unfortunately, that you install slime.

Unfortunately, it no longer works with current slime.

    Debugger entered--Lisp error: (void-function common-lisp-run-indentation-tests)

I can't find a definition of common-lisp-run-indentation-tests anywhere
in slime's code.

>
> which indented the two comment lines to comment-column, I think. The
> patch I attach at the end of this message fixes it.
>
> ...as does indenting the sexp with indent-region. This might be naive,
> but why not super-simplify indent-sexp to be something like this?

Both your patch and indent-region do not indent comments that occur
after code.  But I don't think doing this indentation is a bug, rather,
slime should change `comment-indent-function' and/or
`comment-insert-comment-function' so that comments will be indented in
the way it expects.

>   	;; unless a line ends inside a string.
>   	(while (and (not inner-loop-done)
>   		    (not (setq outer-loop-done (eobp))))
> + 	  (setq before-parse (point))
>   	  (setq state (parse-partial-sexp (point) (progn (end-of-line) (point))
>   					  nil nil state))
>   	  (setq next-depth (car state))
> ! 	  ;; If the line contains nothing but a comment other than the sort that
> ! 	  ;; is indented like code, indent it now with indent-for-comment.
> ! 	  ;; Comments indented like code are right already.  In any case clear
> ! 	  ;; the in-comment flag in the state because parse-partial-sexp never
> ! 	  ;; sees the newlines.
> ! 	  (when (car (nthcdr 4 state))
> !             (when (<= (nth 8 state) before-parse)

I think this would only indent comments starting at the beginning of the
line, which never need to be indented anyway...

> !               (indent-for-comment)
> !               (end-of-line))
> !             (setcar (nthcdr 4 state) nil))

bug#16202: 24.3.50; lisp-mode's indent-sexp might be buggy (for slime in particular, and lisp in general)

[Noam Postavsky]

close 16202
quit

npostavs@users.sourceforge.net writes:

> Both your patch and indent-region do not indent comments that occur
> after code.  But I don't think doing this indentation is a bug, rather,
> slime should change `comment-indent-function' and/or
> `comment-insert-comment-function' so that comments will be indented in
> the way it expects.

I'm therefore closing.