bug#9622: [PATCH] Re: bug#9622: 23.3; flet indentation

[akater <nuclearspace@gmail.com>]

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

Several fixes:

- We now check whether we're in the relevant subexpression of flet and
  friends.

- Added -p to the predicate's name.

- Added cl-macrolet cl-flet* cl-symbol-macrolet to the list of local
  definers.  I added cl-symbol-macrolet too because this is how SLIME
  indents symbol-macrolet.

Technical notes:

- I attached flet-tests.sexp which only contains cl-flet forms written
  the way they are meant to be indented.  I think writing them in
  strings in deftest forms would be quite ugly and unreliable.  If
  appropriate, I could add test(s) that would try to indent individual
  forms in the file; no change indicates test(s) passed.  Only I'm not
  sure what would be the best way to define an individual ert test per
  form but I guess I'll figure it out.  OK?

- parse-partial-sexp does not keep any record of relative position of
  subexpressions in superexpressions which I found surprising.  Things
  would be easier if it did.  Other things might become easier too.
  But I did not dare to patch it.

I also have stylistic notes.  First, the less important ones.

- You changed “iff” to “if”; I meant “iff”, i.e. “if and only if”; I
  now changed it to “when” simply because the semantics corresponds
  directly to that of lisp's own lingo and is thus in my opinion
  easier to read.

- Lines
  > In Wishful Lisp, the following form would be
  > ...
  are supposed to convey the intent of what the form in quistion does,
  in a concise way.  Feel free to remove this if you feel it doesn't
  help.

The more involved stylistic note is

- a line split issue that I wanted to discuss for some time.  See the
  diff when it comes to second-order-parent and (pop parents).  TL;DR:
  it's entirely up to you but I wanted to point out that this was done
  deliberately.  Note that if pop (or a hypothetical cdrf) accepted
  multiple arguments, most people would agree it's fine to keep these
  two on a single line; if you have time, see the discussion I started
  in emacs-devel:
  https://lists.gnu.org/archive/html/emacs-devel/2021-09/msg02189.html


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

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Fix cl-flet indentation --]
[-- Type: text/x-diff, Size: 5317 bytes --]

From 688ec23060f89a75d7356bba576bc8d31f32174f Mon Sep 17 00:00:00 2001
From: akater <nuclearspace@gmail.com>
Date: Wed, 29 Sep 2021 15:45:32 +0000
Subject: [PATCH] ; Fixup: cl-flet indentation

Addresses several deficiencies in bugfix
38037e04cb05cb1f2b604f0b1602d36b0bcf6985

- Check whether we're in the relevant subexpression of flet and
friends.

- Add -p to the predicate's name.

- Add cl-macrolet cl-flet* cl-symbol-macrolet to the list of local
definers.  cl-symbol-macrolet added because this is how SLIME indents
symbol-macrolet.
---
 lisp/emacs-lisp/lisp-mode.el | 55 ++++++++++++++++++++++++++++--------
 1 file changed, 43 insertions(+), 12 deletions(-)

diff --git a/lisp/emacs-lisp/lisp-mode.el b/lisp/emacs-lisp/lisp-mode.el
index 7ce857e990..d2d63606da 100644
--- a/lisp/emacs-lisp/lisp-mode.el
+++ b/lisp/emacs-lisp/lisp-mode.el
@@ -1106,26 +1106,55 @@ defun calculate-lisp-indent (&optional parse-start)
               (t
                normal-indent))))))
 
-(defun lisp--local-defform-body (state)
-  "Return non-nil if at local definition body according to STATE.
+(defun lisp--local-defform-body-p (state)
+  "Return non-nil when at local definition body according to STATE.
 STATE is the `parse-partial-sexp' state for current position."
   (when-let ((start-of-innermost-containing-list (nth 1 state)))
-    (let* ((parents (nth 9 state))
-           (second-cons-after (cddr parents))
-           second-order-parent)
+    (let* ((parents (nth 9 state)) second-order-parent
+           (second-cons-after (cddr parents)))
       (while second-cons-after
         (when (= start-of-innermost-containing-list
                  (car second-cons-after))
           (setq second-order-parent (car parents)
                 ;; Leave the loop.
                 second-cons-after nil))
-        (pop second-cons-after)
-        (pop parents))
+        (pop second-cons-after) (pop parents))
       (and second-order-parent
            (save-excursion
              (goto-char (1+ second-order-parent))
-             (memq (read (current-buffer))
-                   '(cl-flet cl-labels)))))))
+             (and (memq (read (current-buffer))
+                        '( cl-flet cl-labels cl-macrolet cl-flet*
+                           cl-symbol-macrolet))
+                  ;; Now we must check that we are
+                  ;; in the second element of the flet-like form.
+                  ;; It would be easier if `parse-partial-sexp' also recorded
+                  ;; relative positions of subsexps in supersexps
+                  ;; but it doesn't so we check manually.
+                  ;;
+                  ;; First, we must be looking at list now.
+                  (ignore-errors (when (= (scan-lists (point) 1 0)
+                                          (scan-sexps (point) 1))
+                                   ;; Looking at list; descend into it:
+                                   (down-list 1)
+                                   t))
+                  ;; In Wishful Lisp, the following form would be
+                  ;; (cl-member start-of-innermost-containing-list
+                  ;;            (points-at-beginning-of-lists-at-this-level)
+                  ;;            :test #'=)
+                  (cl-loop
+                   with pos = (ignore-errors
+                                ;; The first local definition may be indented
+                                ;; with whitespace following open paren.
+                                (goto-char (scan-lists (point) 1 0))
+                                (goto-char (scan-lists (point) -1 0))
+                                (point))
+                   while pos
+                   do (if (= start-of-innermost-containing-list pos)
+                          (cl-return t)
+                        (setq pos (ignore-errors
+                                    (goto-char (scan-lists (point) 2 0))
+                                    (goto-char (scan-lists (point) -1 0))
+                                    (point)))))))))))
 
 (defun lisp-indent-function (indent-point state)
   "This function is the normal value of the variable `lisp-indent-function'.
@@ -1160,7 +1189,9 @@ defun lisp-indent-function (indent-point state)
     (if (and (elt state 2)
              (not (looking-at "\\sw\\|\\s_")))
         ;; car of form doesn't seem to be a symbol
-        (if (lisp--local-defform-body state)
+        (if (lisp--local-defform-body-p state)
+            ;; We nevertheless check whether we are in flet-like form
+            ;; as we presume local function names could be non-symbols.
             (lisp-indent-defform state indent-point)
           (if (not (> (save-excursion (forward-line 1) (point))
                       calculate-lisp-indent-last-sexp))
@@ -1184,8 +1215,8 @@ defun lisp-indent-function (indent-point state)
 		   (and (null method)
 			(> (length function) 3)
 			(string-match "\\`def" function))
-                   ;; Check whether we are in flet or labels.
-                   (lisp--local-defform-body state))
+                   ;; Check whether we are in flet-like form.
+                   (lisp--local-defform-body-p state))
 	       (lisp-indent-defform state indent-point))
 	      ((integerp method)
 	       (lisp-indent-specform method state
-- 
2.32.0


[-- Attachment #3: Test cases --]
[-- Type: text/plain, Size: 3806 bytes --]

(cl-flet ()
  (a (dangerous-position
      b)))

(cl-flet wrong-syntax-but-should-not-obstruct-indentation
  (a (dangerous-position
      b)))

(cl-flet ((a (arg-of-flet-a)
            b
            c)))

(cl-flet ((a (arg-of-flet-a)
            b
            c
            (if d
                e
              f))
          (irregular-local-def (form
                                returning
                                lambda))
          (g (arg-of--flet-g)
            h
            i))
  (let ((j k))
    (if dangerous-position
        l
      m)))

(cl-flet ((a (arg-of-flet-a)
            b
            c
            (if d
                e
              f))
          (irregular-local-def (form
                                returning
                                lambda))
          (g (arg-of--flet-g)
            h
            i))
  (let ((j k))
    (if dangerous-position
        l
      m)))

(cl-flet ((a (arg-of-flet-a)
            b
            c
            (if d
                e
              f))
          (irregular-local-def (form
                                returning
                                lambda))
          (irregular-local-def (form returning
                                     lambda))
          wrong-syntax-but-should-not-osbtruct-indentation
          (g (arg-of--flet-g)
            h
            i))
  (let ((j k))
    (if dangerous-position
        l
      m)))

(cl-flet ((a (arg-of-flet-a)
            b
            c
            (if d
                e
              f))
          (irregular-local-def (form
                                returning
                                lambda))
          wrong-syntax-but-should-not-osbtruct-indentation
          (g (arg-of--flet-g)
            h
            i))
  (let ((j k))
    (if dangerous-position
        l
      m)))

(cl-flet (wrong-syntax-but-should-not-obstruct-indentation
          (g (arg-of--flet-g)
            h
            i))
  (let ((j k))
    (if dangerous-position
        l
      m)))

;; (setf _) not yet supported but looks like it will be
(cl-flet (((setf a) (new value)
            stuff)
          wrong-syntax-but-should-not-obstruct-indentation
          (g (arg-of--flet-g)
            h
            i))
  (let ((j k))
    (if dangerous-position
        l
      m)))

(cl-flet (   (a (arg-of-flet-a)
               b
               c
               (if d
                   e
                 f))
             (irregular-local-def (form
                                   returning
                                   lambda))
             (g (arg-of--flet-g)
               h
               i))
  (let ((j k))
    (if dangerous-position
        l
      m)))

(cl-flet (   wrong-syntax-but-should-not-obstruct-indentation
             (g (arg-of--flet-g)
               h
               i))
  (let ((j k))
    (if dangerous-position
        l
      m)))

(cl-flet (   wrong-syntax-but-should-not-obstruct-indentation
             (g (arg-of--flet-g)
               h
               i))
  (let ((j k))
    (if dangerous-position
        l
      m)))

(cl-flet (wrong-syntax-but-should-not-obstruct-indentation
          (g (arg-of--flet-g)
            h
            i)
          wrong-syntax-but-should-not-obstruct-indentation
          (g (arg-of--flet-g)
            h
            i)))

(cl-flet (wrong-syntax-but-should-not-obstruct-indentation
          wrong-syntax-but-should-not-obstruct-indentation
          (g (arg-of--flet-g)
            h
            i)
          wrong-syntax-but-should-not-obstruct-indentation))

(cl-flet (wrong-syntax-but-should-not-obstruct-indentation
          wrong-syntax-but-should-not-obstruct-indentation
          wrong-syntax-but-should-not-obstruct-indentation
          (g (arg-of--flet-g)
            h
            i)))