* bug#9622: [PATCH] Re: bug#9622: 23.3; flet indentation
[not found] <504153FB-8633-4755-A91A-DF5DD64E6FAA@acm.org>
2021-09-28 3:01 ` akater
@ 2021-09-29 18:12 ` akater
2021-11-08 1:13 ` Michael Heerdegen
1 sibling, 1 reply; 9+ messages in thread
From: akater @ 2021-09-29 18:12 UTC (permalink / raw)
To: Mattias Engdegård; +Cc: Lars Ingebrigtsen, 9622, Diogo F. S. Ramos
[-- 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)))
^ permalink raw reply related [flat|nested] 9+ messages in thread