bug#36552: 25.1; lisp tagbody indentation not working

bug#36552: 25.1; lisp tagbody indentation not working

[Ryan Brown]

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

I'm typing in some old lisp code that makes heavy use of (prog) and (go).
I was trying to figure out how to get  emacs to format the tagbody labels
like the original source listing:
(PROG (X Y Z)
   (...)
 LOOP
   (...)
   (GO LOOP) )

I discovered that the common lisp indentation is supposed to support it,
but it wasn't working.
The first line of the prog gets properly indented as either an expression
or a tag, but then every following line just gets the same indentation as
the previous one.

Sure enough, lisp-indent-259 only only runs the indentation function for
the first item after &rest:

          (cond ((and tail (not (symbolp tem)))
                 ;; indent tail of &rest in same way as first elt of rest
                 (throw 'exit normal-indent))
With this change the tagbodies get properly indented:
          (cond ((and tail (not (or (consp tem) (symbolp tem))))
                 ;; indent tail of &rest in same way as first elt of rest
                 (throw 'exit normal-indent))

Here's the entire patched function.
(defun lisp-indent-259
    (method path state indent-point sexp-column normal-indent)
  (catch 'exit
    (let ((p path)
          (containing-form-start (elt state 1))
          n tem tail)
      ;; Isn't tail-recursion wonderful?
      (while p
        ;; This while loop is for destructuring.
        ;; p is set to (cdr p) each iteration.
        (if (not (consp method)) (lisp-indent-report-bad-format method))
        (setq n (1- (car p))
              p (cdr p)
              tail nil)
        (while n
          ;; This while loop is for advancing along a method
          ;; until the relevant (possibly &rest/&body) pattern
          ;; is reached.
          ;; n is set to (1- n) and method to (cdr method)
          ;; each iteration.
          (setq tem (car method))

          (or (eq tem 'nil)             ;default indentation
              (eq tem '&lambda)         ;lambda list
              (and (eq tem '&body) (null (cdr method)))
              (and (eq tem '&rest)
                   (consp (cdr method))
                   (null (cddr method)))
              (integerp tem)            ;explicit indentation specified
              (and (consp tem)          ;destructuring
                   (eq (car tem) '&whole)
                   (or (symbolp (cadr tem))
                       (integerp (cadr tem))))
              (and (symbolp tem)        ;a function to call to do the work.
                   (null (cdr method)))
              (lisp-indent-report-bad-format method))

          (cond ((and tail (not (or (consp tem) (symbolp tem))))
                 ;; indent tail of &rest in same way as first elt of rest
                 (throw 'exit normal-indent))
                ((eq tem '&body)
                 ;; &body means (&rest <lisp-body-indent>)
                 (throw 'exit
                   (if (and (= n 0)     ;first body form
                            (null p))   ;not in subforms
                       (+ sexp-column
                          lisp-body-indent)
                       normal-indent)))
                ((eq tem '&rest)
                 ;; this pattern holds for all remaining forms
                 (setq tail (> n 0)
                       n 0
                       method (cdr method)))
                ((> n 0)
                 ;; try next element of pattern
                 (setq n (1- n)
                       method (cdr method))
                 (if (< n 0)
                     ;; Too few elements in pattern.
                     (throw 'exit normal-indent)))
                ((eq tem 'nil)
(throw 'exit (if (consp normal-indent)
 normal-indent
(list normal-indent containing-form-start))))
((eq tem '&lambda)
(throw 'exit
(cond ((null p)
      (list (+ sexp-column 4) containing-form-start))
     ((null (cdr p))
                               ;; Indentation within a lambda-list. -- dvl
                               (list (lisp-indent-lambda-list
                                      indent-point
                                      sexp-column
                                      containing-form-start)
                                     containing-form-start))
                              (t
                               normal-indent))))
                ((integerp tem)
                 (throw 'exit
                   (if (null p)         ;not in subforms
                       (list (+ sexp-column tem) containing-form-start)
                       normal-indent)))
                ((symbolp tem)          ;a function to call
                 (throw 'exit
                   (funcall tem path state indent-point
                            sexp-column normal-indent)))
                (t
                 ;; must be a destructing frob
                 (if (not (null p))
                     ;; descend
               (setq method (cddr tem)
                           n nil)
               (setq tem (cadr tem))
                   (throw 'exit
                     (cond (tail
                            normal-indent)
                           ((eq tem 'nil)
                            (list normal-indent
                                  containing-form-start))
                           ((integerp tem)
                            (list (+ sexp-column tem)
                                  containing-form-start))
                           (t
                            (funcall tem path state indent-point
                                     sexp-column normal-indent))))))))))))


In GNU Emacs 25.1.1 (x86_64-pc-linux-gnu)
 of 2017-09-14, modified by Debian built on trouble
System Description: Debian GNU/Linux 9.8 (stretch)

Configured using:
 'configure --build x86_64-linux-gnu --prefix=/usr
 --sharedstatedir=/var/lib --libexecdir=/usr/lib
 --localstatedir=/var/lib --infodir=/usr/share/info
 --mandir=/usr/share/man --with-pop=yes
 --enable-locallisppath=/etc/emacs25:/etc/emacs:/usr/local/share/emacs/25.1/site-lisp:/usr/local/share/emacs/site-lisp:/usr/share/emacs/25.1/site-lisp:/usr/share/emacs/site-lisp
 --with-sound=alsa --build x86_64-linux-gnu --prefix=/usr
 --sharedstatedir=/var/lib --libexecdir=/usr/lib
 --localstatedir=/var/lib --infodir=/usr/share/info
 --mandir=/usr/share/man --with-pop=yes
 --enable-locallisppath=/etc/emacs25:/etc/emacs:/usr/local/share/emacs/25.1/site-lisp:/usr/local/share/emacs/site-lisp:/usr/share/emacs/25.1/site-lisp:/usr/share/emacs/site-lisp
 --with-sound=alsa --with-x=no --without-gconf --without-gsettings
 'CFLAGS=-g -O2
 -fdebug-prefix-map=/build/emacs25-wN2qS3/emacs25-25.1+1=.
-fstack-protector-strong
 -Wformat -Werror=format-security -Wall' 'CPPFLAGS=-Wdate-time
 -D_FORTIFY_SOURCE=2' LDFLAGS=-Wl,-z,relro'

Configured features:
JPEG SOUND GPM DBUS NOTIFY ACL LIBSELINUX GNUTLS LIBXML2 ZLIB

Important settings:
  value of $LANG: en_US.UTF-8
  locale-coding-system: utf-8-unix

Major mode: Lisp

Minor modes in effect:
  tooltip-mode: t
  global-eldoc-mode: t
  electric-indent-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  line-number-mode: t
  transient-mark-mode: t

Recent messages:
Auto-saving...
Mark set
Auto-saving...done
Auto-saving...done
next-line: End of buffer
Auto-saving...done
Mark set [2 times]
Auto-saving...done
Saving file /home/ryan/sin.lisp...
Wrote /home/ryan/sin.lisp

Load-path shadows:
/usr/share/emacs/25.1/site-lisp/debian-startup hides
/usr/share/emacs/site-lisp/debian-startup

Features:
(shadow sort mail-extr emacsbug message dired format-spec rfc822 mml
mml-sec password-cache epg epg-config gnus-util mm-decode mm-bodies
mm-encode mail-parse rfc2231 mailabbrev gmm-utils mailheader sendmail
rfc2047 rfc2045 ietf-drums mm-util mail-prsvr mail-utils cl-extra
thingatpt help-fns help-mode tool-bar cus-edit cus-start cus-load
wid-edit regexp-opt gv find-func edebug easymenu cl-loaddefs pcase
cl-lib misearch multi-isearch jka-compr term/xterm xterm time-date
cl-indent mule-util tooltip eldoc electric uniquify ediff-hook vc-hooks
lisp-float-type tabulated-list newcomment elisp-mode lisp-mode prog-mode
register page menu-bar rfn-eshadow timer select mouse jit-lock font-lock
syntax facemenu font-core frame cl-generic cham georgian utf-8-lang
misc-lang vietnamese tibetan thai tai-viet lao korean japanese eucjp-ms
cp51932 hebrew greek romanian slovak czech european ethiopic indian
cyrillic chinese charscript case-table epa-hook jka-cmpr-hook help
simple abbrev minibuffer cl-preloaded nadvice loaddefs button faces
cus-face macroexp files text-properties overlay sha1 md5 base64 format
env code-pages mule custom widget hashtable-print-readable backquote
dbusbind inotify multi-tty make-network-process emacs)

Memory information:
((conses 16 124690 4205)
 (symbols 48 20939 0)
 (miscs 40 122 580)
 (strings 32 20140 4691)
 (string-bytes 1 550849)
 (vectors 16 11774)
 (vector-slots 8 399804 2164)
 (floats 8 172 959)
 (intervals 56 3273 0)
 (buffers 976 25))

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

bug#36552: 25.1; lisp tagbody indentation not working

[Lars Ingebrigtsen]

Ryan Brown <ryan@derivita.com> writes:

> Sure enough, lisp-indent-259 only only runs the indentation function for the first item after
> &rest:
>
>           (cond ((and tail (not (symbolp tem)))
>                  ;; indent tail of &rest in same way as first elt of rest
>                  (throw 'exit normal-indent))
> With this change the tagbodies get properly indented:
>           (cond ((and tail (not (or (consp tem) (symbolp tem))))
>                  ;; indent tail of &rest in same way as first elt of rest
>                  (throw 'exit normal-indent))
>
> Here's the entire patched function.

Could you send a patch for this instead?

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

bug#36552: 25.1; lisp tagbody indentation not working

[Ryan Brown]

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

Do you want me to just attach a patch here, or is there a process for
sending a patch?
I'm not finding documentation on how to do that.

On Mon, Jul 8, 2019 at 1:53 PM Lars Ingebrigtsen <larsi@gnus.org> wrote:

> Ryan Brown <ryan@derivita.com> writes:
>
> > Sure enough, lisp-indent-259 only only runs the indentation function for
> the first item after
> > &rest:
> >
> >           (cond ((and tail (not (symbolp tem)))
> >                  ;; indent tail of &rest in same way as first elt of rest
> >                  (throw 'exit normal-indent))
> > With this change the tagbodies get properly indented:
> >           (cond ((and tail (not (or (consp tem) (symbolp tem))))
> >                  ;; indent tail of &rest in same way as first elt of rest
> >                  (throw 'exit normal-indent))
> >
> > Here's the entire patched function.
>
> Could you send a patch for this instead?
>
> --
> (domestic pets only, the antidote for overdose, milk.)
>    bloggy blog: http://lars.ingebrigtsen.no
>

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

bug#36552: 25.1; lisp tagbody indentation not working

[Lars Ingebrigtsen]

Ryan Brown <ryan@derivita.com> writes:

> Do you want me to just attach a patch here, or is there a process for
> sending a patch?

Just attach a patch.

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

bug#36552: 25.1; lisp tagbody indentation not working

[Ryan Brown]

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

On Mon, Jul 8, 2019 at 2:06 PM Lars Ingebrigtsen <larsi@gnus.org> wrote:

> Ryan Brown <ryan@derivita.com> writes:
>
> > Do you want me to just attach a patch here, or is there a process for
> > sending a patch?
>
> Just attach a patch.
>
> --
> (domestic pets only, the antidote for overdose, milk.)
>    bloggy blog: http://lars.ingebrigtsen.no
>

[-- Attachment #1.2: Type: text/html, Size: 805 bytes --]

[-- Attachment #2: tagbody.patch --]
[-- Type: application/octet-stream, Size: 1000 bytes --]

From 5a9e5e3996443ed308b87fc31a00c33c50d12e6c Mon Sep 17 00:00:00 2001
From: Ryan Brown <ryan@derivita.com>
Date: Mon, 8 Jul 2019 15:30:25 -0600
Subject: [PATCH] Fix for lisp tagbody indentation

---
 lisp/emacs-lisp/cl-indent.el | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lisp/emacs-lisp/cl-indent.el b/lisp/emacs-lisp/cl-indent.el
index 10af440008d444216e3ace96cd4a2d8d99c991ca..ad5f31713aff16b9ab1e196af02a6910a2b141dd 100644
--- a/lisp/emacs-lisp/cl-indent.el
+++ b/lisp/emacs-lisp/cl-indent.el
@@ -593,7 +593,7 @@ optional\\|rest\\|key\\|allow-other-keys\\|aux\\|whole\\|body\\|environment\
                    (null (cdr method)))
               (lisp-indent-report-bad-format method))
 
-          (cond ((and tail (not (consp tem)))
+          (cond ((and tail (not (or (consp tem) (symbolp tem))))
                  ;; indent tail of &rest in same way as first elt of rest
                  (throw 'exit normal-indent))
                 ((eq tem '&body)
-- 
2.11.0

bug#36552: 25.1; lisp tagbody indentation not working

[Lars Ingebrigtsen]

Ryan Brown <ryan@derivita.com> writes:

> -          (cond ((and tail (not (consp tem)))
> +          (cond ((and tail (not (or (consp tem) (symbolp tem))))

Thanks.  I was, however, unable to see any difference in how PROG was
indented.  Do you have a test case, starting from emacs -Q that displays
the problem and that's fixed by this patch?

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

bug#36552: 25.1; lisp tagbody indentation not working

[Ryan Brown]

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

M-X eval-expression (require 'cl-indent)
M-X set-variable lisp-indent-function common-lisp-indent-function
C-X f test.lisp
(PROG (A)
(foo)
BAR
(go BAR) )

The BAR should be indented less than the sexps:
(PROG (A)
   (foo)
 BAR
   (go BAR) )

On Mon, Jul 8, 2019 at 4:07 PM Lars Ingebrigtsen <larsi@gnus.org> wrote:

> Ryan Brown <ryan@derivita.com> writes:
>
> > -          (cond ((and tail (not (consp tem)))
> > +          (cond ((and tail (not (or (consp tem) (symbolp tem))))
>
> Thanks.  I was, however, unable to see any difference in how PROG was
> indented.  Do you have a test case, starting from emacs -Q that displays
> the problem and that's fixed by this patch?
>
> --
> (domestic pets only, the antidote for overdose, milk.)
>    bloggy blog: http://lars.ingebrigtsen.no
>

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

bug#36552: 25.1; lisp tagbody indentation not working

[Lars Ingebrigtsen]

Ryan Brown <ryan@derivita.com> writes:

> M-X eval-expression (require 'cl-indent)
> M-X set-variable lisp-indent-function common-lisp-indent-function
> C-X f test.lisp
> (PROG (A)
> (foo)
> BAR
> (go BAR) )

Thanks; I'm now able to reproduce this (and verify that your patch fixes
this).  There doesn't seem to be extensive test coverage of the
indentation stuff, so I'm not quite confident that it breaks anything
else or not, but I'm applying, and I guess we'll see...

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