* bug#9622: 23.3; flet indentation [not found] <504153FB-8633-4755-A91A-DF5DD64E6FAA@acm.org> @ 2021-09-28 3:01 ` akater 2021-09-28 5:23 ` Lars Ingebrigtsen 2021-09-29 18:12 ` bug#9622: [PATCH] " akater 1 sibling, 1 reply; 36+ messages in thread From: akater @ 2021-09-28 3:01 UTC (permalink / raw) To: Mattias Engdegård; +Cc: Lars Ingebrigtsen, 9622 [-- Attachment #1: Type: text/plain, Size: 134 bytes --] I'm not around Emacs 28 atm; could you check if it's still wrong when cl-flet's definition — square (x) — is not on a single line? [-- Attachment #2: Type: text/html, Size: 175 bytes --] ^ permalink raw reply [flat|nested] 36+ messages in thread
* bug#9622: 23.3; flet indentation 2021-09-28 3:01 ` bug#9622: 23.3; flet indentation akater @ 2021-09-28 5:23 ` Lars Ingebrigtsen 2021-09-28 10:11 ` Mattias Engdegård ` (2 more replies) 0 siblings, 3 replies; 36+ messages in thread From: Lars Ingebrigtsen @ 2021-09-28 5:23 UTC (permalink / raw) To: akater; +Cc: Mattias Engdegård, 9622 akater <nuclearspace@gmail.com> writes: > I'm not around Emacs 28 atm; could you check if it's still wrong when > cl-flet's definition — square (x) — is not on a single line? Yes, it's still wrongly indented: (cl-flet ((square (x) (* x x))) (let* ((a 3) (b (square a))) ; <-- malindented b)) -- (domestic pets only, the antidote for overdose, milk.) bloggy blog: http://lars.ingebrigtsen.no ^ permalink raw reply [flat|nested] 36+ messages in thread
* bug#9622: 23.3; flet indentation 2021-09-28 5:23 ` Lars Ingebrigtsen @ 2021-09-28 10:11 ` Mattias Engdegård 2021-09-28 10:35 ` Mattias Engdegård 2021-09-28 16:39 ` akater 2 siblings, 0 replies; 36+ messages in thread From: Mattias Engdegård @ 2021-09-28 10:11 UTC (permalink / raw) To: Lars Ingebrigtsen; +Cc: 9622, akater It affects other constructs too: (cl-flet ((f () alpha)) (cond (beta gamma))) ; <- wrong ^ permalink raw reply [flat|nested] 36+ messages in thread
* bug#9622: 23.3; flet indentation 2021-09-28 5:23 ` Lars Ingebrigtsen 2021-09-28 10:11 ` Mattias Engdegård @ 2021-09-28 10:35 ` Mattias Engdegård 2021-09-28 16:39 ` akater 2 siblings, 0 replies; 36+ messages in thread From: Mattias Engdegård @ 2021-09-28 10:35 UTC (permalink / raw) To: Lars Ingebrigtsen; +Cc: 9622, akater More examples: (cl-flet ((f () alpha)) (let ((x y)) (if beta gamma ; <- wrong delta))) (cl-flet ((f () alpha)) (gamma (delta epsilon))) ; <- wrong ^ permalink raw reply [flat|nested] 36+ messages in thread
* bug#9622: 23.3; flet indentation 2021-09-28 5:23 ` Lars Ingebrigtsen 2021-09-28 10:11 ` Mattias Engdegård 2021-09-28 10:35 ` Mattias Engdegård @ 2021-09-28 16:39 ` akater 2 siblings, 0 replies; 36+ messages in thread From: akater @ 2021-09-28 16:39 UTC (permalink / raw) To: Lars Ingebrigtsen; +Cc: Mattias Engdegård, 9622 [-- Attachment #1: Type: text/plain, Size: 126 bytes --] OK I found the culprit; figuring out the least expensive way to resolve. One way or another, I'll fix it in the coming days. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 865 bytes --] ^ permalink raw reply [flat|nested] 36+ messages in thread
* bug#9622: [PATCH] Re: bug#9622: 23.3; flet indentation [not found] <504153FB-8633-4755-A91A-DF5DD64E6FAA@acm.org> 2021-09-28 3:01 ` bug#9622: 23.3; flet indentation akater @ 2021-09-29 18:12 ` akater 2021-09-30 6:37 ` Lars Ingebrigtsen 2021-11-08 1:13 ` Michael Heerdegen 1 sibling, 2 replies; 36+ 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] 36+ messages in thread
* bug#9622: [PATCH] Re: bug#9622: 23.3; flet indentation 2021-09-29 18:12 ` bug#9622: [PATCH] " akater @ 2021-09-30 6:37 ` Lars Ingebrigtsen 2021-09-30 8:05 ` Mattias Engdegård 2021-09-30 13:06 ` Basil L. Contovounesios via Bug reports for GNU Emacs, the Swiss army knife of text editors 2021-11-08 1:13 ` Michael Heerdegen 1 sibling, 2 replies; 36+ messages in thread From: Lars Ingebrigtsen @ 2021-09-30 6:37 UTC (permalink / raw) To: akater; +Cc: Mattias Engdegård, 9622, Diogo F. S. Ramos akater <nuclearspace@gmail.com> writes: > - 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. Thanks; applied to Emacs 28. > - 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. Indeed. I have a new machinery for doing tests like this waiting for Emacs 29 development to commence to fix this awkwardness. > - 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. Oops, sorry. > 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 We never put two statements on the same line like that, so I added a newline there (and some other stylistic fixes, like changing the `and' to a when (the `and' is logical here (since we're doing a predicate), but we prefer `when' when it's a long and involved body). -- (domestic pets only, the antidote for overdose, milk.) bloggy blog: http://lars.ingebrigtsen.no ^ permalink raw reply [flat|nested] 36+ messages in thread
* bug#9622: [PATCH] Re: bug#9622: 23.3; flet indentation 2021-09-30 6:37 ` Lars Ingebrigtsen @ 2021-09-30 8:05 ` Mattias Engdegård 2021-09-30 13:06 ` Basil L. Contovounesios via Bug reports for GNU Emacs, the Swiss army knife of text editors 1 sibling, 0 replies; 36+ messages in thread From: Mattias Engdegård @ 2021-09-30 8:05 UTC (permalink / raw) To: Lars Ingebrigtsen; +Cc: 9622, akater 30 sep. 2021 kl. 08.37 skrev Lars Ingebrigtsen <larsi@gnus.org>: >> - 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. What would be unreliable? > Indeed. I have a new machinery for doing tests like this waiting for > Emacs 29 development to commence to fix this awkwardness. We should insist on tests accompanying any bug fix from the start. Awkward is better than nothing. If the author finds it too difficult to write the test then we should help. Akater, we have plenty of indentation tests for other programming modes already and they employ a variety of methods; there are many examples to look at. For instance, look at ruby-mode-tests.el; it contains a converted legacy test (ruby--indent/converted-from-manual-test) that reindents a separate source file, much like your example. If you need further directions, we will assist. ^ permalink raw reply [flat|nested] 36+ messages in thread
* bug#9622: [PATCH] Re: bug#9622: 23.3; flet indentation 2021-09-30 6:37 ` Lars Ingebrigtsen 2021-09-30 8:05 ` Mattias Engdegård @ 2021-09-30 13:06 ` Basil L. Contovounesios via Bug reports for GNU Emacs, the Swiss army knife of text editors 2021-09-30 13:41 ` Lars Ingebrigtsen ` (2 more replies) 1 sibling, 3 replies; 36+ messages in thread From: Basil L. Contovounesios via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2021-09-30 13:06 UTC (permalink / raw) To: Lars Ingebrigtsen; +Cc: Mattias Engdegård, 9622, akater, Diogo F. S. Ramos Lars Ingebrigtsen [2021-09-30 08:37 +0200] wrote: > akater <nuclearspace@gmail.com> writes: > >> - 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. > > Thanks; applied to Emacs 28. Thanks, but I found another indentation regression introduced in 38037e04cb that does not involve flet and is not fixed by c42af5aee7: 0. emacs -Q 1. (let ((x (and y RET Point is now at BOL instead of under y. -- Basil ^ permalink raw reply [flat|nested] 36+ messages in thread
* bug#9622: [PATCH] Re: bug#9622: 23.3; flet indentation 2021-09-30 13:06 ` Basil L. Contovounesios via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2021-09-30 13:41 ` Lars Ingebrigtsen 2021-09-30 13:57 ` akater 2021-09-30 14:52 ` Mattias Engdegård 2021-09-30 19:28 ` miha--- via Bug reports for GNU Emacs, the Swiss army knife of text editors 2 siblings, 1 reply; 36+ messages in thread From: Lars Ingebrigtsen @ 2021-09-30 13:41 UTC (permalink / raw) To: Basil L. Contovounesios Cc: Mattias Engdegård, 9622, akater, Diogo F. S. Ramos "Basil L. Contovounesios" <contovob@tcd.ie> writes: > Thanks, but I found another indentation regression introduced in > 38037e04cb that does not involve flet and is not fixed by c42af5aee7: > > 0. emacs -Q > 1. (let ((x (and y RET > > Point is now at BOL instead of under y. Perhaps we should consider backing out these changed in Emacs 28 and continuing to work on them on the trunk? -- (domestic pets only, the antidote for overdose, milk.) bloggy blog: http://lars.ingebrigtsen.no ^ permalink raw reply [flat|nested] 36+ messages in thread
* bug#9622: [PATCH] Re: bug#9622: 23.3; flet indentation 2021-09-30 13:41 ` Lars Ingebrigtsen @ 2021-09-30 13:57 ` akater 2021-10-01 11:27 ` Lars Ingebrigtsen 0 siblings, 1 reply; 36+ messages in thread From: akater @ 2021-09-30 13:57 UTC (permalink / raw) To: Lars Ingebrigtsen, Basil L. Contovounesios Cc: Mattias Engdegård, 9622, Diogo F. S. Ramos [-- Attachment #1.1: Type: text/plain, Size: 421 bytes --] Lars Ingebrigtsen <larsi@gnus.org> writes: > Perhaps we should consider backing out these changed in Emacs 28 and > continuing to work on them on the trunk? I don't think so. The first attempt was written in a hurry, and merged without extensive testing; in the second I did exhaustive tests, it just never crossed my mind to test on unbalanced expressions. This very much looks like the last ignore-errors of them: [-- 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 indentation regression --] [-- Type: text/x-diff, Size: 1691 bytes --] From 2e5f87b345d349dc6657f8034102a842d0685232 Mon Sep 17 00:00:00 2001 From: akater <nuclearspace@gmail.com> Date: Thu, 30 Sep 2021 13:37:59 +0000 Subject: [PATCH] Fix other regressions in cl-flet indentation * lisp/emacs-lisp/lisp-mode.el (lisp--local-defform-body-p): Fix indentation regression introduced by 38037e04cb05cb1f2b604f0b1602d36b0bcf6985 (bug#9622), namely don't fail in unreadable (incl. unbalanced) sexps. --- lisp/emacs-lisp/lisp-mode.el | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/lisp/emacs-lisp/lisp-mode.el b/lisp/emacs-lisp/lisp-mode.el index cd054801bc..57196dfec4 100644 --- a/lisp/emacs-lisp/lisp-mode.el +++ b/lisp/emacs-lisp/lisp-mode.el @@ -1124,9 +1124,13 @@ defun lisp--local-defform-body-p (state) (when second-order-parent (save-excursion (goto-char (1+ second-order-parent)) - (and (memq (read (current-buffer)) - '( cl-flet cl-labels cl-macrolet cl-flet* - cl-symbol-macrolet)) + (and (when-let ((head (ignore-errors + ;; FIXME: This does not distinguish + ;; between reading nil and a read error. + ;; We don't care but still, better fix this. + (read (current-buffer))))) + (memq head '( 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 -- 2.32.0 ^ permalink raw reply related [flat|nested] 36+ messages in thread
* bug#9622: [PATCH] Re: bug#9622: 23.3; flet indentation 2021-09-30 13:57 ` akater @ 2021-10-01 11:27 ` Lars Ingebrigtsen 2021-10-09 7:26 ` akater 0 siblings, 1 reply; 36+ messages in thread From: Lars Ingebrigtsen @ 2021-10-01 11:27 UTC (permalink / raw) To: akater Cc: Basil L. Contovounesios, Mattias Engdegård, 9622, Diogo F. S. Ramos akater <nuclearspace@gmail.com> writes: > I don't think so. The first attempt was written in a hurry, and merged > without extensive testing; in the second I did exhaustive tests, it just > never crossed my mind to test on unbalanced expressions. This very much > looks like the last ignore-errors of them: Thanks, but I think we're just too late to get this into Emacs 28, so I resurrected your patch series and pushed it as one patch to the trunk. (I also added all the test cases from your second patch to ert.) Please have a look and see if I missed something when I did the merge. -- (domestic pets only, the antidote for overdose, milk.) bloggy blog: http://lars.ingebrigtsen.no ^ permalink raw reply [flat|nested] 36+ messages in thread
* bug#9622: [PATCH] Re: bug#9622: 23.3; flet indentation 2021-10-01 11:27 ` Lars Ingebrigtsen @ 2021-10-09 7:26 ` akater 2021-10-09 11:23 ` Lars Ingebrigtsen 0 siblings, 1 reply; 36+ messages in thread From: akater @ 2021-10-09 7:26 UTC (permalink / raw) To: Lars Ingebrigtsen Cc: Basil L. Contovounesios, Mattias Engdegård, 9622, Diogo F. S. Ramos [-- Attachment #1: Type: text/plain, Size: 3141 bytes --] Lars Ingebrigtsen <larsi@gnus.org> writes: > akater <nuclearspace@gmail.com> writes: > >> I don't think so. The first attempt was written in a hurry, and merged >> without extensive testing; in the second I did exhaustive tests, it just >> never crossed my mind to test on unbalanced expressions. This very much >> looks like the last ignore-errors of them: > > Thanks, but I think we're just too late to get this into Emacs 28, so I > resurrected your patch series and pushed it as one patch to the trunk. > (I also added all the test cases from your second patch to ert.) > > Please have a look and see if I missed something when I did the merge. Everything from my last patch is in its place (my branch is even with master, and 15 tests are there) but: We need to also include partial sexps, and a test should ensure RET brings point to the proper position. I have not familiarized myself with this test routine yet so I'm not sure how to do this right. The important aspect I want to point out now is, especially since previous partial sexps affect the indentation of all consequent sexps, we must ensure this will not affect both the “RET tests” and tests where the region is indented. This presumes all our test cases will reside in a single file but they should be because otherwise it will be terrible. This is important enough so I write this message in advance even though the indentation is sometimes wrong and I've not yet figured it out. Here go “RET test cases”; looks like they are exhaustive, modulo variations with longer excessive whitespace. Not everything of what follows is indented correctly with my patch. I'm investigating; some issues might be present even without my patch. In what follows, ^ points to the appropriate position after RET is pressed. The last case that broke my patch: (let ((x (and y ^ A variation of it, just in case: (let ((x ^ ; N.B.: This is the way it is in elisp-mode right now but in lisp-mode, the point is one step further. 5 cases with some whitespace missing: flet-missing-whitespace-1 (cl-flet((f (x) ^ flet-missing-whitespace-2 (cl-flet((f(x) ^ flet-missing-whitespace-3 (cl-flet ((f(x) ^ flet-missing-whitespace-4 (cl-flet( (f (x) ^ flet-missing-whitespace-5 (cl-flet( (f(x) ^ Combinations of missing and excessive whitespace flet-missing-and-excessive-whitespace-1 (cl-flet((f (x) ^ flet-missing-and-excessive-whitespace-2 (cl-flet ((f(x) ^ flet-missing-and-excessive-whitespace-3 (cl-flet( (f (x) ^ flet-missing-and-excessive-whitespace-4 (cl-flet( (f (x) ^ flet-missing-and-excessive-whitespace-5 (cl-flet( (f (x) ^ flet-missing-and-excessive-whitespace-6 (cl-flet( (f(x) ^ There's another problem: in some of these cases, even though initial indentation is incorrect, once the sexp is completed, it is indented correctly. But in some other cases, it's not! I'm looking into this. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 865 bytes --] ^ permalink raw reply [flat|nested] 36+ messages in thread
* bug#9622: [PATCH] Re: bug#9622: 23.3; flet indentation 2021-10-09 7:26 ` akater @ 2021-10-09 11:23 ` Lars Ingebrigtsen 2021-10-28 19:23 ` akater 0 siblings, 1 reply; 36+ messages in thread From: Lars Ingebrigtsen @ 2021-10-09 11:23 UTC (permalink / raw) To: akater Cc: Basil L. Contovounesios, Mattias Engdegård, 9622, Diogo F. S. Ramos akater <nuclearspace@gmail.com> writes: > In what follows, ^ points to the appropriate position after RET is pressed. > > The last case that broke my patch: > (let ((x (and y > > ^ I think these tests should be fine for erts-mode. Just put Point-Char: ^ into the header and make the test be (newline). -- (domestic pets only, the antidote for overdose, milk.) bloggy blog: http://lars.ingebrigtsen.no ^ permalink raw reply [flat|nested] 36+ messages in thread
* bug#9622: [PATCH] Re: bug#9622: 23.3; flet indentation 2021-10-09 11:23 ` Lars Ingebrigtsen @ 2021-10-28 19:23 ` akater 2021-10-28 21:53 ` Lars Ingebrigtsen 0 siblings, 1 reply; 36+ messages in thread From: akater @ 2021-10-28 19:23 UTC (permalink / raw) To: Lars Ingebrigtsen Cc: Basil L. Contovounesios, Mattias Engdegård, 9622, Diogo F. S. Ramos [-- Attachment #1.1: Type: text/plain, Size: 132 bytes --] New patch supports incomplete sexps and adds tests for them. “test-identation” ert test in elisp-mode-tests.el does pass. [-- Attachment #1.2: signature.asc --] [-- Type: application/pgp-signature, Size: 865 bytes --] [-- Attachment #2: Support incomplete sexps when indenting cl-flet-like forms --] [-- Type: text/x-diff, Size: 8078 bytes --] From c3edf97a4f39044885a8bc544a31762ba9a28695 Mon Sep 17 00:00:00 2001 From: akater <nuclearspace@gmail.com> Date: Mon, 18 Oct 2021 03:08:06 +0000 Subject: [PATCH] Indent cl-flet-like forms correctly in incomplete expressions * lisp/emacs-lisp/lisp-mode.el (lisp--local-defform-body-p): Support incomplete sexps * test/lisp/progmodes/elisp-mode-resources/flet.erts: Add tests for incomplete sexps --- lisp/emacs-lisp/lisp-mode.el | 74 +++++------ .../progmodes/elisp-mode-resources/flet.erts | 121 ++++++++++++++++++ 2 files changed, 153 insertions(+), 42 deletions(-) diff --git a/lisp/emacs-lisp/lisp-mode.el b/lisp/emacs-lisp/lisp-mode.el index 5dfb1fae356..15afdef0252 100644 --- a/lisp/emacs-lisp/lisp-mode.el +++ b/lisp/emacs-lisp/lisp-mode.el @@ -1114,56 +1114,46 @@ defun lisp--local-defform-body-p (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) + (first-cons-after (cdr parents)) + (second-cons-after (cdr first-cons-after)) + first-order-parent second-order-parent) (while second-cons-after (when (= start-of-innermost-containing-list (car second-cons-after)) - (setq second-order-parent (car parents) + (setq second-order-parent (pop parents) + first-order-parent (pop parents) ;; Leave the loop. second-cons-after nil)) (pop second-cons-after) (pop parents)) (when second-order-parent - (save-excursion - (goto-char (1+ second-order-parent)) - (and (when-let ((head (ignore-errors - ;; FIXME: This does not distinguish - ;; between reading nil and a read error. - ;; We don't care but still, better fix this. - (read (current-buffer))))) - (memq head '( 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))))))))))) + (let (local-definitions-starting-point) + (and (save-excursion + (goto-char (1+ second-order-parent)) + (when-let ((head (ignore-errors + ;; FIXME: This does not distinguish + ;; between reading nil and a read error. + ;; We don't care but still, better fix this. + (read (current-buffer))))) + (when (memq head '( cl-flet cl-labels cl-macrolet cl-flet* + cl-symbol-macrolet)) + ;; In what follows, we rely on (point) returning non-nil. + (setq local-definitions-starting-point + (progn + (parse-partial-sexp + (point) first-order-parent nil + ;; From docstring of `parse-partial-sexp': + ;; Fourth arg non-nil means stop + ;; when we come to any character + ;; that starts a sexp. + t) + (point)))))) + (ignore-errors + ;; We rely on `backward-up-list' working + ;; even when sexp is incomplete “to the right”. + (backward-up-list 2) + t) + (= local-definitions-starting-point (point)))))))) (defun lisp-indent-function (indent-point state) "This function is the normal value of the variable `lisp-indent-function'. diff --git a/test/lisp/progmodes/elisp-mode-resources/flet.erts b/test/lisp/progmodes/elisp-mode-resources/flet.erts index 447cf08cc25..7c4a0f304e9 100644 --- a/test/lisp/progmodes/elisp-mode-resources/flet.erts +++ b/test/lisp/progmodes/elisp-mode-resources/flet.erts @@ -220,3 +220,124 @@ Name: flet15 h i))) =-=-= + +Name: flet-indentation-incomplete-sexp-no-side-effects-1 +Code: (lambda () (emacs-lisp-mode) (setq indent-tabs-mode nil) (newline nil t)) +Point-Char: | + +=-= +(let ((x (and y| +=-= +(let ((x (and y + | +=-=-= + +Name: flet-indentation-incomplete-sexp-no-side-effects-2 + +=-= +(let ((x| +=-= +(let ((x + | +=-=-= + +Name: flet-indentation-incomplete-sexp-missing-whitespace-1 +Point-Char: | + +=-= +(cl-flet((f (x)| +=-= +(cl-flet((f (x) + | +=-=-= + +Name: flet-indentation-incomplete-sexp-missing-whitespace-2 +Point-Char: | + +=-= +(cl-flet((f(x)| +=-= +(cl-flet((f(x) + | +=-=-= + +Name: flet-indentation-incomplete-sexp-missing-whitespace-3 + +=-= +(cl-flet ((f(x)| +=-= +(cl-flet ((f(x) + | +=-=-= + +Name: flet-indentation-incomplete-sexp-missing-whitespace-4 + +=-= +(cl-flet( (f (x)| +=-= +(cl-flet( (f (x) + | +=-=-= + +Name: flet-indentation-incomplete-sexp-missing-whitespace-5 + +=-= +(cl-flet( (f(x)| +=-= +(cl-flet( (f(x) + | +=-=-= + +Name: flet-indentation-incomplete-sexp-missing-and-excessive-whitespace-1 + +=-= +(cl-flet((f (x)| +=-= +(cl-flet((f (x) + | +=-=-= + +Name: flet-indentation-incomplete-sexp-missing-and-excessive-whitespace-2 + +=-= +(cl-flet ((f(x)| +=-= +(cl-flet ((f(x) + | +=-=-= + +Name: flet-indentation-incomplete-sexp-missing-and-excessive-whitespace-3 + +=-= +(cl-flet( (f (x)| +=-= +(cl-flet( (f (x) + | +=-=-= + +Name: flet-indentation-incomplete-sexp-missing-and-excessive-whitespace-4 + +=-= +(cl-flet( (f (x)| +=-= +(cl-flet( (f (x) + | +=-=-= + +Name: flet-indentation-incomplete-sexp-missing-and-excessive-whitespace-5 + +=-= +(cl-flet( (f (x)| +=-= +(cl-flet( (f (x) + | +=-=-= + +Name: flet-indentation-incomplete-sexp-missing-and-excessive-whitespace-6 + +=-= +(cl-flet( (f(x)| +=-= +(cl-flet( (f(x) + | +=-=-= -- 2.32.0 ^ permalink raw reply related [flat|nested] 36+ messages in thread
* bug#9622: [PATCH] Re: bug#9622: 23.3; flet indentation 2021-10-28 19:23 ` akater @ 2021-10-28 21:53 ` Lars Ingebrigtsen 0 siblings, 0 replies; 36+ messages in thread From: Lars Ingebrigtsen @ 2021-10-28 21:53 UTC (permalink / raw) To: akater Cc: Basil L. Contovounesios, Mattias Engdegård, 9622, Diogo F. S. Ramos akater <nuclearspace@gmail.com> writes: > New patch supports incomplete sexps and adds tests for them. > > “test-identation” ert test in elisp-mode-tests.el does pass. Thanks; seems to work well, so this is now pushed to the trunk. -- (domestic pets only, the antidote for overdose, milk.) bloggy blog: http://lars.ingebrigtsen.no ^ permalink raw reply [flat|nested] 36+ messages in thread
* bug#9622: [PATCH] Re: bug#9622: 23.3; flet indentation 2021-09-30 13:06 ` Basil L. Contovounesios via Bug reports for GNU Emacs, the Swiss army knife of text editors 2021-09-30 13:41 ` Lars Ingebrigtsen @ 2021-09-30 14:52 ` Mattias Engdegård 2021-09-30 15:11 ` Basil L. Contovounesios via Bug reports for GNU Emacs, the Swiss army knife of text editors 2021-09-30 19:28 ` miha--- via Bug reports for GNU Emacs, the Swiss army knife of text editors 2 siblings, 1 reply; 36+ messages in thread From: Mattias Engdegård @ 2021-09-30 14:52 UTC (permalink / raw) To: Basil L. Contovounesios; +Cc: Lars Ingebrigtsen, 9622, akater 30 sep. 2021 kl. 15.06 skrev Basil L. Contovounesios <contovob@tcd.ie>: > Thanks, but I found another indentation regression introduced in > 38037e04cb that does not involve flet and is not fixed by c42af5aee7: > > 0. emacs -Q > 1. (let ((x (and y RET > > Point is now at BOL instead of under y. Thanks for reporting this; I've noticed the same. It's sufficient annoying that I'm reverting both changes; at least that gets us back where we started. Akater, please don't see this as a hostile move -- we all want the bug resolved. Let's make sure that the next patch comes with ERT tests for all the mentioned issues and more. ^ permalink raw reply [flat|nested] 36+ messages in thread
* bug#9622: [PATCH] Re: bug#9622: 23.3; flet indentation 2021-09-30 14:52 ` Mattias Engdegård @ 2021-09-30 15:11 ` Basil L. Contovounesios via Bug reports for GNU Emacs, the Swiss army knife of text editors 2021-09-30 15:23 ` Thierry Volpiatto 2021-09-30 15:25 ` Mattias Engdegård 0 siblings, 2 replies; 36+ messages in thread From: Basil L. Contovounesios via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2021-09-30 15:11 UTC (permalink / raw) To: Mattias Engdegård; +Cc: Lars Ingebrigtsen, 9622, akater Mattias Engdegård [2021-09-30 16:52 +0200] wrote: > 30 sep. 2021 kl. 15.06 skrev Basil L. Contovounesios <contovob@tcd.ie>: > >> Thanks, but I found another indentation regression introduced in >> 38037e04cb that does not involve flet and is not fixed by c42af5aee7: >> >> 0. emacs -Q >> 1. (let ((x (and y RET >> >> Point is now at BOL instead of under y. > > Thanks for reporting this; I've noticed the same. It's sufficient annoying that > I'm reverting both changes; at least that gets us back where we started. Modulo a failing test ;). -- Basil ^ permalink raw reply [flat|nested] 36+ messages in thread
* bug#9622: [PATCH] Re: bug#9622: 23.3; flet indentation 2021-09-30 15:11 ` Basil L. Contovounesios via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2021-09-30 15:23 ` Thierry Volpiatto 2021-09-30 15:33 ` akater 2021-09-30 15:25 ` Mattias Engdegård 1 sibling, 1 reply; 36+ messages in thread From: Thierry Volpiatto @ 2021-09-30 15:23 UTC (permalink / raw) To: 9622; +Cc: contovob, mattiase, larsi, nuclearspace "Basil L. Contovounesios" via "Bug reports for GNU Emacs, the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org> writes: > Mattias Engdegård [2021-09-30 16:52 +0200] wrote: > >> 30 sep. 2021 kl. 15.06 skrev Basil L. Contovounesios <contovob@tcd.ie>: >> >>> Thanks, but I found another indentation regression introduced in >>> 38037e04cb that does not involve flet and is not fixed by c42af5aee7: >>> >>> 0. emacs -Q >>> 1. (let ((x (and y RET >>> >>> Point is now at BOL instead of under y. >> >> Thanks for reporting this; I've noticed the same. It's sufficient annoying that >> I'm reverting both changes; at least that gets us back where we started. > > Modulo a failing test ;). Note that using `common-lisp-indent-function` as `lisp-indent-function` in emacs-27.2 make `cl-flet` and friends indent properly, with a few more settings for `cl-loop` indentation it is perfect, did I miss something? -- Thierry ^ permalink raw reply [flat|nested] 36+ messages in thread
* bug#9622: [PATCH] Re: bug#9622: 23.3; flet indentation 2021-09-30 15:23 ` Thierry Volpiatto @ 2021-09-30 15:33 ` akater 2021-09-30 16:04 ` Thierry Volpiatto 0 siblings, 1 reply; 36+ messages in thread From: akater @ 2021-09-30 15:33 UTC (permalink / raw) To: thievol, 9622; +Cc: contovob, mattiase, larsi [-- Attachment #1: Type: text/plain, Size: 907 bytes --] Thierry Volpiatto <thievol@posteo.net> writes: > Note that using `common-lisp-indent-function` as `lisp-indent-function` in > emacs-27.2 make `cl-flet` and friends indent properly, with a few more > settings for `cl-loop` indentation it is perfect, did I miss something? In general, the languages are different and rules should be expected to be different. cl- indenter takes care of #(..) and so on which is meaningless in Elisp. I also think common-lisp-indent-function should not attempt to indent cl- stuff and likewise lisp-indent-function should not indent plain flet and so on because plain ones are “officially deprecated”. It would make more sense to have a generic sexp indenter and dialect-specific indenters that inherit from it. But inheritance practices in Emacs is in terrible shape: inheritance mechanisms have been reinvented in incompatible ways since forever. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 865 bytes --] ^ permalink raw reply [flat|nested] 36+ messages in thread
* bug#9622: [PATCH] Re: bug#9622: 23.3; flet indentation 2021-09-30 15:33 ` akater @ 2021-09-30 16:04 ` Thierry Volpiatto 0 siblings, 0 replies; 36+ messages in thread From: Thierry Volpiatto @ 2021-09-30 16:04 UTC (permalink / raw) To: akater; +Cc: contovob, mattiase, larsi, 9622 akater <nuclearspace@gmail.com> writes: > Thierry Volpiatto <thievol@posteo.net> writes: > >> Note that using `common-lisp-indent-function` as `lisp-indent-function` in >> emacs-27.2 make `cl-flet` and friends indent properly, with a few more >> settings for `cl-loop` indentation it is perfect, did I miss something? > > In general, the languages are different and rules should be expected to > be different. cl- indenter takes care of #(..) and so on which is > meaningless in Elisp. > > I also think common-lisp-indent-function should not attempt to indent > cl- stuff and likewise lisp-indent-function should not indent plain flet > and so on because plain ones are “officially deprecated”. > > It would make more sense to have a generic sexp indenter and > dialect-specific indenters that inherit from it. But inheritance > practices in Emacs is in terrible shape: inheritance mechanisms have > been reinvented in incompatible ways since forever. Thanks for explanations, make sense. -- Thierry ^ permalink raw reply [flat|nested] 36+ messages in thread
* bug#9622: [PATCH] Re: bug#9622: 23.3; flet indentation 2021-09-30 15:11 ` Basil L. Contovounesios via Bug reports for GNU Emacs, the Swiss army knife of text editors 2021-09-30 15:23 ` Thierry Volpiatto @ 2021-09-30 15:25 ` Mattias Engdegård 2021-09-30 15:56 ` akater 1 sibling, 1 reply; 36+ messages in thread From: Mattias Engdegård @ 2021-09-30 15:25 UTC (permalink / raw) To: Basil L. Contovounesios; +Cc: Lars Ingebrigtsen, 9622, akater 30 sep. 2021 kl. 17.11 skrev Basil L. Contovounesios <contovob@tcd.ie>: > Modulo a failing test ;). Now marked as an expected failure. Thank you! ^ permalink raw reply [flat|nested] 36+ messages in thread
* bug#9622: [PATCH] Re: bug#9622: 23.3; flet indentation 2021-09-30 15:25 ` Mattias Engdegård @ 2021-09-30 15:56 ` akater 0 siblings, 0 replies; 36+ messages in thread From: akater @ 2021-09-30 15:56 UTC (permalink / raw) To: Mattias Engdegård; +Cc: Lars Ingebrigtsen, 9622 [-- Attachment #1: Type: text/plain, Size: 539 bytes --] I don't see this as hostile. I didn't expect my original one-off suggestion to be merged this soon anyway. But I still think we were going the most efficient way of producing the aforementioned “and more” tests. Lars, when the method is established to use external file like flet-tests.sexp (with the last unbalanced example and its variations to be added) rather than strings, let me know, I'll write the tests. As much as I want local definers indented correctly, getting these strings into tests verbatim is too much. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 865 bytes --] ^ permalink raw reply [flat|nested] 36+ messages in thread
* bug#9622: [PATCH] Re: bug#9622: 23.3; flet indentation 2021-09-30 13:06 ` Basil L. Contovounesios via Bug reports for GNU Emacs, the Swiss army knife of text editors 2021-09-30 13:41 ` Lars Ingebrigtsen 2021-09-30 14:52 ` Mattias Engdegård @ 2021-09-30 19:28 ` miha--- via Bug reports for GNU Emacs, the Swiss army knife of text editors 2021-09-30 23:23 ` akater 2 siblings, 1 reply; 36+ messages in thread From: miha--- via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2021-09-30 19:28 UTC (permalink / raw) To: Basil L. Contovounesios, Lars Ingebrigtsen Cc: Mattias Engdegård, 9622, akater, Diogo F. S. Ramos [-- Attachment #1: Type: text/plain, Size: 414 bytes --] > Thanks, but I found another indentation regression introduced in > 38037e04cb that does not involve flet and is not fixed by c42af5aee7: > > 0. emacs -Q > 1. (let ((x (and y RET > I also found a regression that you might want to add as a test case: on Emacs commit c006286780, Evaluating the following form would signal an "Invalid read syntax" error: (pp `[((1 (okno . ,(current-buffer))))]) Best regards. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 861 bytes --] ^ permalink raw reply [flat|nested] 36+ messages in thread
* bug#9622: [PATCH] Re: bug#9622: 23.3; flet indentation 2021-09-30 19:28 ` miha--- via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2021-09-30 23:23 ` akater 0 siblings, 0 replies; 36+ messages in thread From: akater @ 2021-09-30 23:23 UTC (permalink / raw) To: 9622; +Cc: miha Thank you. This is already covered by the last patch. ^ permalink raw reply [flat|nested] 36+ messages in thread
* bug#9622: 23.3; flet indentation 2021-09-29 18:12 ` bug#9622: [PATCH] " akater 2021-09-30 6:37 ` Lars Ingebrigtsen @ 2021-11-08 1:13 ` Michael Heerdegen 2021-11-08 6:18 ` bug#9622: [PATCH] " akater 1 sibling, 1 reply; 36+ messages in thread From: Michael Heerdegen @ 2021-11-08 1:13 UTC (permalink / raw) To: 9622; +Cc: Mattias Engdegård, Lars Ingebrigtsen, akater, Diogo F. S. Ramos akater <nuclearspace@gmail.com> writes: > Several fixes: > [...] Am I allowed to report a regression that might be caused by these changes here? I have code like this: #+begin_src emacs-lisp (cl-flet* ((cont-w/expr (f) (g-w/extended-expr cont f)) (cont-prepend (&rest exps) (cont-w/expr (lambda (p) (apply #'g--and (append exps (list p))))))) (pcase expr ((and (let transformed (g--trivial-transformation-p expr)) (guard transformed)) (cont-prepend transformed)))) #+end_src and the last line gets indented wrongly (one space too less). When I remove the wrapping `cl-flet*' indentation behavior gets back to the expected. Michael. ^ permalink raw reply [flat|nested] 36+ messages in thread
* bug#9622: [PATCH] Re: bug#9622: 23.3; flet indentation 2021-11-08 1:13 ` Michael Heerdegen @ 2021-11-08 6:18 ` akater 2021-11-08 6:38 ` Lars Ingebrigtsen 0 siblings, 1 reply; 36+ messages in thread From: akater @ 2021-11-08 6:18 UTC (permalink / raw) To: Michael Heerdegen, 9622 Cc: Mattias Engdegård, Lars Ingebrigtsen, Diogo F. S. Ramos [-- Attachment #1.1: Type: text/plain, Size: 708 bytes --] Michael Heerdegen <michael_heerdegen@web.de> writes: > #+begin_src emacs-lisp > (cl-flet* ((cont-w/expr (f) (g-w/extended-expr cont f)) > (cont-prepend (&rest exps) > (cont-w/expr (lambda (p) (apply #'g--and (append exps (list p))))))) > (pcase expr > ((and (let transformed (g--trivial-transformation-p expr)) (guard transformed)) > (cont-prepend transformed)))) > #+end_src > > and the last line gets indented wrongly (one space too less). When I > remove the wrapping `cl-flet*' indentation behavior gets back to the > expected. Indeed, I forgot one save-excursion. To Lars: tests involving complete sexps do pass, the rest I couldn't run, as described in bug#51680 [-- Attachment #1.2: signature.asc --] [-- Type: application/pgp-signature, Size: 865 bytes --] [-- Attachment #2: Fix flet indentation --] [-- Type: text/x-diff, Size: 2399 bytes --] From 367ec048e7fbf6d7f3453a64e953079575444f4d Mon Sep 17 00:00:00 2001 From: akater <nuclearspace@gmail.com> Date: Mon, 8 Nov 2021 04:48:13 +0000 Subject: [PATCH] ; * lisp/emacs-lisp/lisp-mode.el: Fix parser state corruption. * lisp/emacs-lisp/lisp-mode.el (lisp--local-defform-body-p): Preserve the point * test/lisp/progmodes/elisp-mode-resources/flet.erts: Add corresponding test example --- lisp/emacs-lisp/lisp-mode.el | 13 +++++++------ test/lisp/progmodes/elisp-mode-resources/flet.erts | 10 ++++++++++ 2 files changed, 17 insertions(+), 6 deletions(-) diff --git a/lisp/emacs-lisp/lisp-mode.el b/lisp/emacs-lisp/lisp-mode.el index 15afdef0252..a5613e70e0a 100644 --- a/lisp/emacs-lisp/lisp-mode.el +++ b/lisp/emacs-lisp/lisp-mode.el @@ -1148,12 +1148,13 @@ defun lisp--local-defform-body-p (state) ;; that starts a sexp. t) (point)))))) - (ignore-errors - ;; We rely on `backward-up-list' working - ;; even when sexp is incomplete “to the right”. - (backward-up-list 2) - t) - (= local-definitions-starting-point (point)))))))) + (save-excursion + (ignore-errors + ;; We rely on `backward-up-list' working + ;; even when sexp is incomplete “to the right”. + (backward-up-list 2) + t) + (= local-definitions-starting-point (point))))))))) (defun lisp-indent-function (indent-point state) "This function is the normal value of the variable `lisp-indent-function'. diff --git a/test/lisp/progmodes/elisp-mode-resources/flet.erts b/test/lisp/progmodes/elisp-mode-resources/flet.erts index 7c4a0f304e9..da3dcb6ec3e 100644 --- a/test/lisp/progmodes/elisp-mode-resources/flet.erts +++ b/test/lisp/progmodes/elisp-mode-resources/flet.erts @@ -221,6 +221,16 @@ Name: flet15 i))) =-=-= +Name: flet16 + +=-= +(cl-flet ((f (x) + (g x))) + (pcase e + ((dangerous-expression) + (form)))) +=-=-= + Name: flet-indentation-incomplete-sexp-no-side-effects-1 Code: (lambda () (emacs-lisp-mode) (setq indent-tabs-mode nil) (newline nil t)) Point-Char: | -- 2.32.0 ^ permalink raw reply related [flat|nested] 36+ messages in thread
* bug#9622: [PATCH] Re: bug#9622: 23.3; flet indentation 2021-11-08 6:18 ` bug#9622: [PATCH] " akater @ 2021-11-08 6:38 ` Lars Ingebrigtsen 2021-11-08 6:38 ` akater 2021-11-08 16:30 ` Michael Heerdegen 0 siblings, 2 replies; 36+ messages in thread From: Lars Ingebrigtsen @ 2021-11-08 6:38 UTC (permalink / raw) To: akater; +Cc: Michael Heerdegen, Mattias Engdegård, 9622, Diogo F. S. Ramos akater <nuclearspace@gmail.com> writes: > Indeed, I forgot one save-excursion. Thanks; this fixes Michael's test case here, too, so I've now pushed it to the trunk. -- (domestic pets only, the antidote for overdose, milk.) bloggy blog: http://lars.ingebrigtsen.no ^ permalink raw reply [flat|nested] 36+ messages in thread
* bug#9622: [PATCH] Re: bug#9622: 23.3; flet indentation 2021-11-08 6:38 ` Lars Ingebrigtsen @ 2021-11-08 6:38 ` akater 2021-11-08 6:53 ` Lars Ingebrigtsen 2021-11-08 16:30 ` Michael Heerdegen 1 sibling, 1 reply; 36+ messages in thread From: akater @ 2021-11-08 6:38 UTC (permalink / raw) To: Lars Ingebrigtsen; +Cc: 9622 [-- Attachment #1: Type: text/plain, Size: 137 bytes --] Sorry: even though half of the tests pass, I did not preserve one check with the last patch and it better be preserved just to be safe: [-- Attachment #2: Fix flet indntation --] [-- Type: text/x-diff, Size: 2432 bytes --] From 2f35789d67115ccd7288ce0b759d6b52c08dde4c Mon Sep 17 00:00:00 2001 From: akater <nuclearspace@gmail.com> Date: Mon, 8 Nov 2021 04:48:13 +0000 Subject: [PATCH] ; * lisp/emacs-lisp/lisp-mode.el: Fix parser state corruption. * lisp/emacs-lisp/lisp-mode.el (lisp--local-defform-body-p): Preserve the point * test/lisp/progmodes/elisp-mode-resources/flet.erts: Add corresponding test example --- lisp/emacs-lisp/lisp-mode.el | 13 +++++++------ test/lisp/progmodes/elisp-mode-resources/flet.erts | 10 ++++++++++ 2 files changed, 17 insertions(+), 6 deletions(-) diff --git a/lisp/emacs-lisp/lisp-mode.el b/lisp/emacs-lisp/lisp-mode.el index 15afdef0252..d90d0f5f6ac 100644 --- a/lisp/emacs-lisp/lisp-mode.el +++ b/lisp/emacs-lisp/lisp-mode.el @@ -1148,12 +1148,13 @@ defun lisp--local-defform-body-p (state) ;; that starts a sexp. t) (point)))))) - (ignore-errors - ;; We rely on `backward-up-list' working - ;; even when sexp is incomplete “to the right”. - (backward-up-list 2) - t) - (= local-definitions-starting-point (point)))))))) + (save-excursion + (when (ignore-errors + ;; We rely on `backward-up-list' working + ;; even when sexp is incomplete “to the right”. + (backward-up-list 2) + t) + (= local-definitions-starting-point (point)))))))))) (defun lisp-indent-function (indent-point state) "This function is the normal value of the variable `lisp-indent-function'. diff --git a/test/lisp/progmodes/elisp-mode-resources/flet.erts b/test/lisp/progmodes/elisp-mode-resources/flet.erts index 7c4a0f304e9..da3dcb6ec3e 100644 --- a/test/lisp/progmodes/elisp-mode-resources/flet.erts +++ b/test/lisp/progmodes/elisp-mode-resources/flet.erts @@ -221,6 +221,16 @@ Name: flet15 i))) =-=-= +Name: flet16 + +=-= +(cl-flet ((f (x) + (g x))) + (pcase e + ((dangerous-expression) + (form)))) +=-=-= + Name: flet-indentation-incomplete-sexp-no-side-effects-1 Code: (lambda () (emacs-lisp-mode) (setq indent-tabs-mode nil) (newline nil t)) Point-Char: | -- 2.32.0 ^ permalink raw reply related [flat|nested] 36+ messages in thread
* bug#9622: [PATCH] Re: bug#9622: 23.3; flet indentation 2021-11-08 6:38 ` akater @ 2021-11-08 6:53 ` Lars Ingebrigtsen 2021-11-08 9:36 ` akater 0 siblings, 1 reply; 36+ messages in thread From: Lars Ingebrigtsen @ 2021-11-08 6:53 UTC (permalink / raw) To: akater; +Cc: 9622 akater <nuclearspace@gmail.com> writes: > Sorry: even though half of the tests pass, I did not preserve one check > with the last patch and it better be preserved just to be safe: Can you send a new patch against the current trunk? -- (domestic pets only, the antidote for overdose, milk.) bloggy blog: http://lars.ingebrigtsen.no ^ permalink raw reply [flat|nested] 36+ messages in thread
* bug#9622: [PATCH] Re: bug#9622: 23.3; flet indentation 2021-11-08 6:53 ` Lars Ingebrigtsen @ 2021-11-08 9:36 ` akater 2021-11-09 3:25 ` Lars Ingebrigtsen 0 siblings, 1 reply; 36+ messages in thread From: akater @ 2021-11-08 9:36 UTC (permalink / raw) To: Lars Ingebrigtsen; +Cc: 9622 [-- Attachment #1: Type: text/plain, Size: 302 bytes --] Lars Ingebrigtsen <larsi@gnus.org> writes: > akater <nuclearspace@gmail.com> writes: > >> Sorry: even though half of the tests pass, I did not preserve one check >> with the last patch and it better be preserved just to be safe: > > Can you send a new patch against the current trunk? Here it goes. [-- Attachment #2: Fix flet indntation --] [-- Type: text/x-diff, Size: 1598 bytes --] From 2a988ef49e1ea954a99e4c123301165764feebac Mon Sep 17 00:00:00 2001 From: akater <nuclearspace@gmail.com> Date: Mon, 8 Nov 2021 04:48:13 +0000 Subject: [PATCH] ; * lisp/emacs-lisp/lisp-mode.el: Restore one check. * lisp/emacs-lisp/lisp-mode.el (lisp--local-defform-body-p): Do check backward-up-list executing without errors --- lisp/emacs-lisp/lisp-mode.el | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/lisp/emacs-lisp/lisp-mode.el b/lisp/emacs-lisp/lisp-mode.el index a5613e70e0a..d90d0f5f6ac 100644 --- a/lisp/emacs-lisp/lisp-mode.el +++ b/lisp/emacs-lisp/lisp-mode.el @@ -1149,12 +1149,12 @@ defun lisp--local-defform-body-p (state) t) (point)))))) (save-excursion - (ignore-errors - ;; We rely on `backward-up-list' working - ;; even when sexp is incomplete “to the right”. - (backward-up-list 2) - t) - (= local-definitions-starting-point (point))))))))) + (when (ignore-errors + ;; We rely on `backward-up-list' working + ;; even when sexp is incomplete “to the right”. + (backward-up-list 2) + t) + (= local-definitions-starting-point (point)))))))))) (defun lisp-indent-function (indent-point state) "This function is the normal value of the variable `lisp-indent-function'. -- 2.32.0 ^ permalink raw reply related [flat|nested] 36+ messages in thread
* bug#9622: [PATCH] Re: bug#9622: 23.3; flet indentation 2021-11-08 9:36 ` akater @ 2021-11-09 3:25 ` Lars Ingebrigtsen 0 siblings, 0 replies; 36+ messages in thread From: Lars Ingebrigtsen @ 2021-11-09 3:25 UTC (permalink / raw) To: akater; +Cc: 9622 akater <nuclearspace@gmail.com> writes: >> Can you send a new patch against the current trunk? > > Here it goes. Thanks; applied to Emacs 29. -- (domestic pets only, the antidote for overdose, milk.) bloggy blog: http://lars.ingebrigtsen.no ^ permalink raw reply [flat|nested] 36+ messages in thread
* bug#9622: [PATCH] Re: bug#9622: 23.3; flet indentation 2021-11-08 6:38 ` Lars Ingebrigtsen 2021-11-08 6:38 ` akater @ 2021-11-08 16:30 ` Michael Heerdegen 1 sibling, 0 replies; 36+ messages in thread From: Michael Heerdegen @ 2021-11-08 16:30 UTC (permalink / raw) To: Lars Ingebrigtsen; +Cc: 9622, akater Lars Ingebrigtsen <larsi@gnus.org> writes: > Thanks; this fixes Michael's test case here, too, so I've now pushed > it to the trunk. Thanks, too -- works now. Michael. ^ permalink raw reply [flat|nested] 36+ messages in thread
* bug#9622: 23.3; flet indentation @ 2011-09-28 1:56 Diogo F. S. Ramos 2019-08-10 20:10 ` Stefan Kangas 2021-09-25 1:29 ` Lars Ingebrigtsen 0 siblings, 2 replies; 36+ messages in thread From: Diogo F. S. Ramos @ 2011-09-28 1:56 UTC (permalink / raw) To: 9622 Currently, when using `flet', the functions are indented with respect of the arglist. It would be nice if flet functions were indented like a `defun', for example. Current behavior: (defun foo () (flet ((long-function-name () (misses-flet))) (long-function-name))) Wanted behavior: (defun foo () (flet ((long-function-name () (hi-flet))) (long-function-name))) -- Diogo F. S. Ramos ^ permalink raw reply [flat|nested] 36+ messages in thread
* bug#9622: 23.3; flet indentation 2011-09-28 1:56 Diogo F. S. Ramos @ 2019-08-10 20:10 ` Stefan Kangas 2021-09-25 1:29 ` Lars Ingebrigtsen 1 sibling, 0 replies; 36+ messages in thread From: Stefan Kangas @ 2019-08-10 20:10 UTC (permalink / raw) To: diogofsr; +Cc: 9622 diogofsr@gmail.com (Diogo F. S. Ramos) writes: > Currently, when using `flet', the functions are indented with respect of > the arglist. > > It would be nice if flet functions were indented like a `defun', for > example. > > Current behavior: > > (defun foo () > (flet ((long-function-name () > (misses-flet))) > (long-function-name))) > > Wanted behavior: > > (defun foo () > (flet ((long-function-name () > (hi-flet))) > (long-function-name))) I can confirm that this is still an issue on Emacs 26.2 and current master. Thanks, Stefan Kangas ^ permalink raw reply [flat|nested] 36+ messages in thread
* bug#9622: 23.3; flet indentation 2011-09-28 1:56 Diogo F. S. Ramos 2019-08-10 20:10 ` Stefan Kangas @ 2021-09-25 1:29 ` Lars Ingebrigtsen 1 sibling, 0 replies; 36+ messages in thread From: Lars Ingebrigtsen @ 2021-09-25 1:29 UTC (permalink / raw) To: Diogo F. S. Ramos; +Cc: 9622 diogofsr@gmail.com (Diogo F. S. Ramos) writes: > Currently, when using `flet', the functions are indented with respect of > the arglist. > > It would be nice if flet functions were indented like a `defun', for > example. This is now fixed in Emacs 28: commit 57dcc26e4af4e4f7812f2e287f18d36c662bf6b1 (HEAD -> master) Author: akater <nuclearspace@gmail.com> AuthorDate: Sat Sep 25 03:27:29 2021 +0200 -- (domestic pets only, the antidote for overdose, milk.) bloggy blog: http://lars.ingebrigtsen.no ^ permalink raw reply [flat|nested] 36+ messages in thread
end of thread, other threads:[~2021-11-09 3:25 UTC | newest] Thread overview: 36+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- [not found] <504153FB-8633-4755-A91A-DF5DD64E6FAA@acm.org> 2021-09-28 3:01 ` bug#9622: 23.3; flet indentation akater 2021-09-28 5:23 ` Lars Ingebrigtsen 2021-09-28 10:11 ` Mattias Engdegård 2021-09-28 10:35 ` Mattias Engdegård 2021-09-28 16:39 ` akater 2021-09-29 18:12 ` bug#9622: [PATCH] " akater 2021-09-30 6:37 ` Lars Ingebrigtsen 2021-09-30 8:05 ` Mattias Engdegård 2021-09-30 13:06 ` Basil L. Contovounesios via Bug reports for GNU Emacs, the Swiss army knife of text editors 2021-09-30 13:41 ` Lars Ingebrigtsen 2021-09-30 13:57 ` akater 2021-10-01 11:27 ` Lars Ingebrigtsen 2021-10-09 7:26 ` akater 2021-10-09 11:23 ` Lars Ingebrigtsen 2021-10-28 19:23 ` akater 2021-10-28 21:53 ` Lars Ingebrigtsen 2021-09-30 14:52 ` Mattias Engdegård 2021-09-30 15:11 ` Basil L. Contovounesios via Bug reports for GNU Emacs, the Swiss army knife of text editors 2021-09-30 15:23 ` Thierry Volpiatto 2021-09-30 15:33 ` akater 2021-09-30 16:04 ` Thierry Volpiatto 2021-09-30 15:25 ` Mattias Engdegård 2021-09-30 15:56 ` akater 2021-09-30 19:28 ` miha--- via Bug reports for GNU Emacs, the Swiss army knife of text editors 2021-09-30 23:23 ` akater 2021-11-08 1:13 ` Michael Heerdegen 2021-11-08 6:18 ` bug#9622: [PATCH] " akater 2021-11-08 6:38 ` Lars Ingebrigtsen 2021-11-08 6:38 ` akater 2021-11-08 6:53 ` Lars Ingebrigtsen 2021-11-08 9:36 ` akater 2021-11-09 3:25 ` Lars Ingebrigtsen 2021-11-08 16:30 ` Michael Heerdegen 2011-09-28 1:56 Diogo F. S. Ramos 2019-08-10 20:10 ` Stefan Kangas 2021-09-25 1:29 ` Lars Ingebrigtsen
Code repositories for project(s) associated with this external index https://git.savannah.gnu.org/cgit/emacs.git https://git.savannah.gnu.org/cgit/emacs/org-mode.git This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.