unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* 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

* 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: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: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: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: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 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 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: [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: 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: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: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         ` 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  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: [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

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 public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).