GNU bug report logs -
#26619
26.0.50; Wrong indentation in emacs-lisp-mode
Previous Next
Reported by: Tino Calancha <tino.calancha <at> gmail.com>
Date: Sun, 23 Apr 2017 07:18:01 UTC
Severity: normal
Tags: fixed, patch
Found in version 26.0.50
Done: npostavs <at> users.sourceforge.net
Bug is archived. No further changes may be made.
To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 26619 in the body.
You can then email your comments to 26619 AT debbugs.gnu.org in the normal way.
Toggle the display of automated, internal messages from the tracker.
Report forwarded
to
npostavs <at> gmail.com, bug-gnu-emacs <at> gnu.org
:
bug#26619
; Package
emacs
.
(Sun, 23 Apr 2017 07:18:01 GMT)
Full text and
rfc822 format available.
Acknowledgement sent
to
Tino Calancha <tino.calancha <at> gmail.com>
:
New bug report received and forwarded. Copy sent to
npostavs <at> gmail.com, bug-gnu-emacs <at> gnu.org
.
(Sun, 23 Apr 2017 07:18:01 GMT)
Full text and
rfc822 format available.
Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):
X-Debbugs-CC: npostavs <at> gmail.com
Write a file /tmp/test.el with contents:
(defun test ()
"This is a test.
Test indentation in emacs-lisp-mode"
(message "Hi!"))
emacs -Q /tmp/test.el
C-x h TAB ; Wrong indentation.
This happens after commit 6fa9cc0593150a318f0e08e69ec10672d548a7c1
; Merge: improve indent-sexp and lisp-indent-region performance
In GNU Emacs 26.0.50 (build 1, x86_64-pc-linux-gnu, GTK+ Version 3.22.11)
of 2017-04-23
Repository revision: b20d05c6d76ddaf7e70da1430c9aac56ef1d6b31
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#26619
; Package
emacs
.
(Sun, 23 Apr 2017 14:50:02 GMT)
Full text and
rfc822 format available.
Message #8 received at 26619 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
tags 26619 patch
quit
Tino Calancha <tino.calancha <at> gmail.com> writes:
> Write a file /tmp/test.el with contents:
> (defun test ()
> "This is a test.
> Test indentation in emacs-lisp-mode"
> (message "Hi!"))
>
> emacs -Q /tmp/test.el
> C-x h TAB ; Wrong indentation.
>
> This happens after commit 6fa9cc0593150a318f0e08e69ec10672d548a7c1
> ; Merge: improve indent-sexp and lisp-indent-region performance
Oops, right. I made lisp-indent-region keep a running parse state like
indent-sexp, but I should have taken the indent-stack too. Here's a
patch:
[v1-0001-Fix-lisp-indent-region-with-multiline-string-lite.patch (text/x-diff, inline)]
From bb640d2f1570b38d98950fb0c374f66ef0988bd2 Mon Sep 17 00:00:00 2001
From: Noam Postavsky <npostavs <at> gmail.com>
Date: Sun, 23 Apr 2017 10:43:05 -0400
Subject: [PATCH v1] Fix lisp-indent-region with multiline string literals
(Bug#26619)
* lisp/emacs-lisp/lisp-mode.el (lisp-mode--indent-cache): New
function, extracted from `indent-sexp'.
(lisp-indent-region, indent-sexp): Use it.
* test/lisp/emacs-lisp/lisp-mode-tests.el
(lisp-mode-tests--correctly-indented-sexp): Extract literal to constant.
(lisp-indent-region, lisp-indent-region-defun-with-docstring): New
tests.
---
lisp/emacs-lisp/lisp-mode.el | 80 ++++++++++++++++++---------------
test/lisp/emacs-lisp/lisp-mode-tests.el | 57 ++++++++++++++++++++---
2 files changed, 97 insertions(+), 40 deletions(-)
diff --git a/lisp/emacs-lisp/lisp-mode.el b/lisp/emacs-lisp/lisp-mode.el
index fa931e76ad..d619dd8f4c 100644
--- a/lisp/emacs-lisp/lisp-mode.el
+++ b/lisp/emacs-lisp/lisp-mode.el
@@ -764,6 +764,36 @@ lisp-ppss
(parse-partial-sexp (car (last (nth 9 pss))) pos)
pss)))
+(defun lisp-mode--indent-cache (init-depth)
+ "Returns a closure that computes indentation, caching by depth."
+ (let ((indent-stack (list nil))
+ (last-depth init-depth))
+ (lambda (&optional depth-or-state)
+ "Pass depth to update cache, or parse state for indentation."
+ (if (listp depth-or-state) ; It's a parse state.
+ (let ((val (if (car indent-stack) indent-stack
+ (calculate-lisp-indent depth-or-state))))
+ (cond ((nth 3 depth-or-state) nil) ; Inside a string.
+ ((integerp val)
+ (setf (car indent-stack) val))
+ ((consp val) ; (COLUMN CONTAINING-SEXP-START)
+ (car val))
+ ;; This only happens if we're in a string.
+ (t (error "This shouldn't happen"))))
+ (let ((depth depth-or-state)) ; It's a depth.
+ (when (< depth init-depth)
+ (setq indent-stack (nconc indent-stack
+ (make-list (- init-depth depth) nil))
+ last-depth (- last-depth depth)
+ depth init-depth))
+ (let ((depth-delta (- depth last-depth)))
+ (cond ((< depth-delta 0)
+ (setq indent-stack (nthcdr (- depth-delta) indent-stack)))
+ ((> depth-delta 0)
+ (setq indent-stack (nconc (make-list depth-delta nil)
+ indent-stack))))
+ (setq last-depth depth)))))))
+
(defun lisp-indent-region (start end)
"Indent region as Lisp code, efficiently."
(save-excursion
@@ -773,12 +803,13 @@ lisp-indent-region
;; parse state, which forces each indent call to reparse from the
;; beginning. That has O(n^2) complexity.
(let* ((parse-state (lisp-ppss start))
+ (calc-indent (lisp-mode--indent-cache (car parse-state)))
(last-syntax-point start)
(pr (unless (minibufferp)
(make-progress-reporter "Indenting region..." (point) end))))
(while (< (point) end)
(unless (and (bolp) (eolp))
- (lisp-indent-line parse-state))
+ (lisp-indent-line (funcall calc-indent parse-state)))
(forward-line 1)
(let ((last-sexp (nth 2 parse-state)))
(setq parse-state (parse-partial-sexp last-syntax-point (point)
@@ -788,16 +819,18 @@ lisp-indent-region
(unless (nth 2 parse-state)
(setf (nth 2 parse-state) last-sexp))
(setq last-syntax-point (point)))
+ ;; Update cache's depth stack.
+ (funcall calc-indent (car parse-state))
(and pr (progress-reporter-update pr (point))))
(and pr (progress-reporter-done pr))
(move-marker end nil))))
-(defun lisp-indent-line (&optional parse-state)
+(defun lisp-indent-line (&optional indent)
"Indent current line as Lisp code."
(interactive)
(let ((pos (- (point-max) (point)))
(indent (progn (beginning-of-line)
- (calculate-lisp-indent (or parse-state (lisp-ppss))))))
+ (or indent (calculate-lisp-indent (lisp-ppss))))))
(skip-chars-forward " \t")
(if (or (null indent) (looking-at "\\s<\\s<\\s<"))
;; Don't alter indentation of a ;;; comment line
@@ -1117,15 +1150,12 @@ indent-sexp
If optional arg ENDPOS is given, indent each line, stopping when
ENDPOS is encountered."
(interactive)
- (let* ((indent-stack (list nil))
- ;; Use `syntax-ppss' to get initial state so we don't get
+ (let* (;; Use `syntax-ppss' to get initial state so we don't get
;; confused by starting inside a string. We don't use
;; `syntax-ppss' in the loop, because this is measurably
;; slower when we're called on a long list.
(state (syntax-ppss))
- (init-depth (car state))
- (next-depth init-depth)
- (last-depth init-depth)
+ (calc-indent (lisp-mode--indent-cache (car state)))
(last-syntax-point (point)))
;; We need a marker because we modify the buffer
;; text preceding endpos.
@@ -1152,48 +1182,28 @@ indent-sexp
(setq last-sexp (or (nth 2 state) last-sexp))
(setq last-syntax-point (point)))
(setf (nth 2 state) last-sexp))
- (setq next-depth (car state))
+ ;; Update cache's depth stack.
+ (funcall calc-indent (car state))
;; If the line contains a comment indent it now with
;; `indent-for-comment'.
(when (nth 4 state)
(indent-for-comment)
(end-of-line))
(setq last-syntax-point (point))
- (when (< next-depth init-depth)
- (setq indent-stack (nconc indent-stack
- (make-list (- init-depth next-depth) nil))
- last-depth (- last-depth next-depth)
- next-depth init-depth))
;; Now indent the next line according to what we learned from
;; parsing the previous one.
(forward-line 1)
(when (< (point) endpos)
- (let ((depth-delta (- next-depth last-depth)))
- (cond ((< depth-delta 0)
- (setq indent-stack (nthcdr (- depth-delta) indent-stack)))
- ((> depth-delta 0)
- (setq indent-stack (nconc (make-list depth-delta nil)
- indent-stack))))
- (setq last-depth next-depth))
;; But not if the line is blank, or just a comment (we
;; already called `indent-for-comment' above).
(skip-chars-forward " \t")
(unless (or (eolp) (eq (char-syntax (char-after)) ?<))
(indent-line-to
- (or (car indent-stack)
- ;; The state here is actually to the end of the
- ;; previous line, but that's fine for our purposes.
- ;; And parsing over the newline would only destroy
- ;; element 2 (last sexp position).
- (let ((val (calculate-lisp-indent state)))
- (cond ((integerp val)
- (setf (car indent-stack) val))
- ((consp val) ; (COLUMN CONTAINING-SEXP-START)
- (car val))
- ;; `calculate-lisp-indent' only returns nil
- ;; when we're in a string, but this won't
- ;; happen because we skip strings above.
- (t (error "This shouldn't happen!"))))))))))
+ ;; The state here is actually to the end of the
+ ;; previous line, but that's fine for our purposes.
+ ;; And parsing over the newline would only destroy
+ ;; element 2 (last sexp position).
+ (funcall calc-indent state))))))
(move-marker endpos nil)))
(defun indent-pp-sexp (&optional arg)
diff --git a/test/lisp/emacs-lisp/lisp-mode-tests.el b/test/lisp/emacs-lisp/lisp-mode-tests.el
index 27f0bb5ec1..3c6364acc8 100644
--- a/test/lisp/emacs-lisp/lisp-mode-tests.el
+++ b/test/lisp/emacs-lisp/lisp-mode-tests.el
@@ -21,10 +21,7 @@
(require 'cl-lib)
(require 'lisp-mode)
-(ert-deftest indent-sexp ()
- "Test basics of \\[indent-sexp]."
- (with-temp-buffer
- (insert "\
+(defconst lisp-mode-tests--correctly-indented-sexp "\
\(a
(prog1
(prog1
@@ -42,9 +39,14 @@
2) ; comment
;; comment
b)")
+
+(ert-deftest indent-sexp ()
+ "Test basics of \\[indent-sexp]."
+ (with-temp-buffer
+ (insert lisp-mode-tests--correctly-indented-sexp)
(goto-char (point-min))
(let ((indent-tabs-mode nil)
- (correct (buffer-string)))
+ (correct lisp-mode-tests--correctly-indented-sexp))
(dolist (mode '(fundamental-mode emacs-lisp-mode))
(funcall mode)
(indent-sexp)
@@ -97,5 +99,50 @@
(indent-sexp)
(should (equal (buffer-string) correct)))))
+(ert-deftest lisp-indent-region ()
+ "Test basics of `lisp-indent-region'."
+ (with-temp-buffer
+ (insert lisp-mode-tests--correctly-indented-sexp)
+ (goto-char (point-min))
+ (let ((indent-tabs-mode nil)
+ (correct lisp-mode-tests--correctly-indented-sexp))
+ (emacs-lisp-mode)
+ (indent-region (point-min) (point-max))
+ ;; Don't mess up correctly indented code.
+ (should (string= (buffer-string) correct))
+ ;; Correctly add indentation.
+ (save-excursion
+ (while (not (eobp))
+ (delete-horizontal-space)
+ (forward-line)))
+ (indent-region (point-min) (point-max))
+ (should (equal (buffer-string) correct))
+ ;; Correctly remove indentation.
+ (save-excursion
+ (let ((n 0))
+ (while (not (eobp))
+ (unless (looking-at "noindent\\|^[[:blank:]]*$")
+ (insert (make-string n ?\s)))
+ (cl-incf n)
+ (forward-line))))
+ (indent-region (point-min) (point-max))
+ (should (equal (buffer-string) correct)))))
+
+
+(ert-deftest lisp-indent-region-defun-with-docstring ()
+ "Test Bug#26619."
+ (with-temp-buffer
+ (insert "\
+\(defun test ()
+ \"This is a test.
+Test indentation in emacs-lisp-mode\"
+ (message \"Hi!\"))")
+ (let ((indent-tabs-mode nil)
+ (correct (buffer-string)))
+ (emacs-lisp-mode)
+ (indent-region (point-min) (point-max))
+ (should (equal (buffer-string) correct)))))
+
+
(provide 'lisp-mode-tests)
;;; lisp-mode-tests.el ends here
--
2.11.1
Added tag(s) patch.
Request was from
npostavs <at> users.sourceforge.net
to
control <at> debbugs.gnu.org
.
(Sun, 23 Apr 2017 14:50:02 GMT)
Full text and
rfc822 format available.
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#26619
; Package
emacs
.
(Mon, 24 Apr 2017 21:04:03 GMT)
Full text and
rfc822 format available.
Message #13 received at 26619 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Hi Noam,
I ended up here as I also noticed a big difference in emacs-lisp
indentation.
Your patch fixes the multi-line string indentation. Thanks!
But indentation is still broken in cases like these:
(with-eval-after-load 'foo
(setq bar `(
baz)))
It instead indents to:
(with-eval-after-load 'foo
(setq bar `(
baz)))
The "b" in "baz" is lining up with "'" in "'foo".
Reverting lisp-mode.el to that in commit 66dc8dd66dc8dd fixes the problem.
--
Kaushal Modi
[Message part 2 (text/html, inline)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#26619
; Package
emacs
.
(Wed, 26 Apr 2017 03:06:02 GMT)
Full text and
rfc822 format available.
Message #16 received at 26619 <at> debbugs.gnu.org (full text, mbox):
Kaushal Modi <kaushal.modi <at> gmail.com> writes:
> But indentation is still broken in cases like these:
>
> (with-eval-after-load 'foo
> (setq bar `(
> baz)))
>
> It instead indents to:
>
> (with-eval-after-load 'foo
> (setq bar `(
> baz)))
There must be more cases. I already get wrong indentation with very
simple cases. E.g. here:
#+begin_src emacs-lisp
(defun test ()
"A test function"
(switch-to-buffer "*Messages*")
17)
#+end_src
Mark the string "*Messages*" and hit C-M-\. The whole line is indented
to a wrong column. While this example is not super useful, calls like
this may happen from Lisp.
Michael.
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#26619
; Package
emacs
.
(Wed, 26 Apr 2017 03:21:01 GMT)
Full text and
rfc822 format available.
Message #19 received at 26619 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Kaushal Modi <kaushal.modi <at> gmail.com> writes:
>
> But indentation is still broken in cases like these:
>
> (with-eval-after-load 'foo
> (setq bar `(
> baz)))
>
Michael Heerdegen <michael_heerdegen <at> web.de> writes:
>
> There must be more cases. I already get wrong indentation with very
> simple cases. E.g. here:
>
> #+begin_src emacs-lisp
> (defun test ()
> "A test function"
> (switch-to-buffer "*Messages*")
> 17)
> #+end_src
Hmm, I was too aggressive about preserving the last sexp position of the
parse state. The following patch fixes both these cases for
indent-region (it applies on top of the previous one), though it needs
to be redone more cleanly.
[v1-0002-WIP-Fix-additional-indentation-problems-Bug-26619.patch (text/x-diff, inline)]
From 101fa466c3e5303458dc6abd2d24ef55d508d9cd Mon Sep 17 00:00:00 2001
From: Noam Postavsky <npostavs <at> gmail.com>
Date: Tue, 25 Apr 2017 23:16:12 -0400
Subject: [PATCH v1 2/2] [WIP] Fix additional indentation problems (Bug#26619)
* lisp/emacs-lisp/lisp-mode.el (lisp-indent-region): Don't use last
sexp from different depths.
---
lisp/emacs-lisp/lisp-mode.el | 34 ++++++++++++++++++++++++----------
1 file changed, 24 insertions(+), 10 deletions(-)
diff --git a/lisp/emacs-lisp/lisp-mode.el b/lisp/emacs-lisp/lisp-mode.el
index d619dd8f4c..12fd53140c 100644
--- a/lisp/emacs-lisp/lisp-mode.el
+++ b/lisp/emacs-lisp/lisp-mode.el
@@ -811,14 +811,25 @@ lisp-indent-region
(unless (and (bolp) (eolp))
(lisp-indent-line (funcall calc-indent parse-state)))
(forward-line 1)
- (let ((last-sexp (nth 2 parse-state)))
- (setq parse-state (parse-partial-sexp last-syntax-point (point)
- nil nil parse-state))
- ;; It's important to preserve last sexp location for
- ;; `calculate-lisp-indent'.
- (unless (nth 2 parse-state)
- (setf (nth 2 parse-state) last-sexp))
- (setq last-syntax-point (point)))
+ (let ((oldstate parse-state)
+ (target-point (point)))
+ (while
+ (progn
+ (setq parse-state (parse-partial-sexp last-syntax-point target-point
+ nil t oldstate))
+ (if (>= (point) target-point)
+ nil ; Done.
+ (when (= (nth 0 parse-state) (nth 0 oldstate)) ; Stopped before open paren.
+ (setq parse-state (parse-partial-sexp last-syntax-point target-point
+ (1+ (nth 0 parse-state)) nil parse-state)))
+ (setq last-syntax-point (point))
+ ;; It's important to preserve last sexp location for
+ ;; `calculate-lisp-indent', but it's only relevant at the
+ ;; same depth.
+ (unless (or (nth 2 parse-state) (/= (nth 0 parse-state) (nth 0 oldstate)))
+ (setf (nth 2 parse-state) (nth 2 oldstate)))
+ t))
+ (setq oldstate parse-state)))
;; Update cache's depth stack.
(funcall calc-indent (car parse-state))
(and pr (progress-reporter-update pr (point))))
@@ -1169,7 +1180,8 @@ indent-sexp
;; Parse this line so we can learn the state to indent the
;; next line. Preserve element 2 of the state (last sexp) for
;; `calculate-lisp-indent'.
- (let ((last-sexp (nth 2 state)))
+ (let ((last-depth (nth 0 state))
+ (last-sexp (nth 2 state)))
(while (progn
(setq state (parse-partial-sexp
last-syntax-point (progn (end-of-line) (point))
@@ -1179,7 +1191,9 @@ indent-sexp
(nth 3 state))
(setq state (parse-partial-sexp (point) (point-max)
nil nil state 'syntax-table))
- (setq last-sexp (or (nth 2 state) last-sexp))
+ (when (nth 2 state)
+ (setq last-sexp (nth 2 state))
+ (setq last-depth (nth 0 state)))
(setq last-syntax-point (point)))
(setf (nth 2 state) last-sexp))
;; Update cache's depth stack.
--
2.11.1
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#26619
; Package
emacs
.
(Wed, 26 Apr 2017 03:30:02 GMT)
Full text and
rfc822 format available.
Message #22 received at 26619 <at> debbugs.gnu.org (full text, mbox):
npostavs <at> users.sourceforge.net writes:
> Hmm, I was too aggressive about preserving the last sexp position of the
> parse state. The following patch fixes both these cases for
> indent-region (it applies on top of the previous one), though it needs
> to be redone more cleanly.
Thanks for the quick response. I'll keep my eyes open and try to test
some more. Could you please send me a cumulative patch relative to
master?
Thanks,
Michael.
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#26619
; Package
emacs
.
(Wed, 26 Apr 2017 03:53:01 GMT)
Full text and
rfc822 format available.
Message #25 received at 26619 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Michael Heerdegen <michael_heerdegen <at> web.de> writes:
> npostavs <at> users.sourceforge.net writes:
>
> Thanks for the quick response. I'll keep my eyes open and try to test
> some more. Could you please send me a cumulative patch relative to
> master?
Sure.
[26619.diff (text/plain, attachment)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#26619
; Package
emacs
.
(Wed, 26 Apr 2017 04:02:02 GMT)
Full text and
rfc822 format available.
Message #28 received at 26619 <at> debbugs.gnu.org (full text, mbox):
npostavs <at> users.sourceforge.net writes:
> Sure.
Thanks. I'll have a look.
Michael.
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#26619
; Package
emacs
.
(Wed, 26 Apr 2017 18:29:01 GMT)
Full text and
rfc822 format available.
Message #31 received at 26619 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
On Tue, Apr 25, 2017 at 11:52 PM <npostavs <at> users.sourceforge.net> wrote:
> Michael Heerdegen <michael_heerdegen <at> web.de> writes:
>
> > npostavs <at> users.sourceforge.net writes:
> >
> > Thanks for the quick response. I'll keep my eyes open and try to test
> > some more. Could you please send me a cumulative patch relative to
> > master?
>
> Sure.
>
Hi Noam,
I tried that patch, but now the indentation is all compressed!
Earlier, the default indentation was 2 spaces, now it is 1.
This is what I see when I reindent one of my files and then undo (with the
help of volatile-highlights package).. Note the lighter grey areas at the
beginning of text on all lines.. the indentation after applying the patch
moved all the text to the left starting in those lighter-grey areas.
[image: pasted1]
--
Kaushal Modi
[Message part 2 (text/html, inline)]
[pasted1 (image/png, inline)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#26619
; Package
emacs
.
(Wed, 26 Apr 2017 22:31:02 GMT)
Full text and
rfc822 format available.
Message #34 received at 26619 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Kaushal Modi <kaushal.modi <at> gmail.com> writes:
>
> I tried that patch, but now the indentation is all compressed!
>
> Earlier, the default indentation was 2 spaces, now it is 1.
Um, right, I didn't test that patch properly, it doesn't work at all.
Here's a fix for it, cumulative diff also attached.
[26619-fixup.diff (text/x-diff, inline)]
--- a/lisp/emacs-lisp/lisp-mode.el
+++ b/lisp/emacs-lisp/lisp-mode.el
@@ -816,20 +816,15 @@ lisp-indent-region
(while
(progn
(setq parse-state (parse-partial-sexp last-syntax-point target-point
- nil t oldstate))
- (if (>= (point) target-point)
- nil ; Done.
- (when (= (nth 0 parse-state) (nth 0 oldstate)) ; Stopped before open paren.
- (setq parse-state (parse-partial-sexp last-syntax-point target-point
- (1+ (nth 0 parse-state)) nil parse-state)))
- (setq last-syntax-point (point))
- ;; It's important to preserve last sexp location for
- ;; `calculate-lisp-indent', but it's only relevant at the
- ;; same depth.
- (unless (or (nth 2 parse-state) (/= (nth 0 parse-state) (nth 0 oldstate)))
- (setf (nth 2 parse-state) (nth 2 oldstate)))
- t))
- (setq oldstate parse-state)))
+ (nth 0 oldstate) nil oldstate))
+ (setq last-syntax-point (point))
+ (< (point) target-point))
+ (setq oldstate parse-state))
+ ;; It's important to preserve last sexp location for
+ ;; `calculate-lisp-indent', but it's only relevant at the
+ ;; same depth.
+ (unless (or (nth 2 parse-state) (/= (nth 0 parse-state) (nth 0 oldstate)))
+ (setf (nth 2 parse-state) (nth 2 oldstate))))
;; Update cache's depth stack.
(funcall calc-indent (car parse-state))
(and pr (progress-reporter-update pr (point))))
[26619-cumulative.diff (text/plain, attachment)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#26619
; Package
emacs
.
(Thu, 27 Apr 2017 11:53:02 GMT)
Full text and
rfc822 format available.
Message #37 received at 26619 <at> debbugs.gnu.org (full text, mbox):
npostavs <at> users.sourceforge.net writes:
> Um, right, I didn't test that patch properly, it doesn't work at all.
> Here's a fix for it, cumulative diff also attached.
Thanks.
Here is a recipe of a problem with that new patch: I have this code:
#+begin_src emacs-lisp
(defun el-search--split (matcher1 matcher2 list)
"Helper for the \"append\" pattern type.
When a splitting of LIST into two lists L1, L2 exist so that Li
is matched by MATCHERi, return (L1 L2) for such Li, else return
nil."
(let ((try-match (lambda (list1 list2)
(when (and (el-search--match-p matcher1 list1)
(el-search--match-p matcher2 list2))
(list list1 list2))))
(list1 list) (list2 '()) (match nil))
;; don't use recursion, this could hit `max-lisp-eval-depth'
(while (and (not (setq match (funcall try-match list1 list2)))
(consp list1))
(let ((last-list1 (last list1)))
(if-let ((cdr-last-list1 (cdr last-list1)))
;; list1 is a dotted list. Then list2 must be empty.
(progn (setcdr last-list1 nil)
(setq list2 cdr-last-list1))
(setq list1 (butlast list1 1)
list2 (cons (car last-list1) list2)))))
match))
#+end_src
I mark the region between the forth line of the defun's body, with point
at the open paren of "(list list1 list2)", and the end of the defun.
Hitting C-M-\ results in all but the first line of the region given an
indentation of zero.
TIA,
Michael.
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#26619
; Package
emacs
.
(Sat, 29 Apr 2017 02:20:02 GMT)
Full text and
rfc822 format available.
Message #40 received at 26619 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
I think I got it this time.
[v3-0001-Fix-lisp-indent-region-and-indent-sexp-Bug-26619.patch (text/x-diff, inline)]
From 4b50ab92dddba5178ed5ea16a93ee5b53fec8f02 Mon Sep 17 00:00:00 2001
From: Noam Postavsky <npostavs <at> gmail.com>
Date: Sun, 23 Apr 2017 10:43:05 -0400
Subject: [PATCH v3] Fix lisp-indent-region and indent-sexp (Bug#26619)
* lisp/emacs-lisp/lisp-mode.el (lisp-ppss): Use an OLDSTATE correct
that corresponds with the start point when calling parse-partial-sexp.
(lisp-indent-state): New struct.
(lisp-indent--next-line): New function.
(indent-sexp, lisp-indent-region): Use it.
(lisp-indent-line): Take indentation, instead of parse state.
* test/lisp/emacs-lisp/lisp-mode-tests.el
(lisp-mode-tests--correctly-indented-sexp): New constant.
(lisp-indent-region, lisp-indent-region-defun-with-docstring):
(lisp-indent-region-open-paren, lisp-indent-region-in-sexp): New
tests.
---
lisp/emacs-lisp/lisp-mode.el | 168 ++++++++++++++++----------------
test/lisp/emacs-lisp/lisp-mode-tests.el | 85 +++++++++++++++-
2 files changed, 165 insertions(+), 88 deletions(-)
diff --git a/lisp/emacs-lisp/lisp-mode.el b/lisp/emacs-lisp/lisp-mode.el
index fa931e76ad..b1e73de5d1 100644
--- a/lisp/emacs-lisp/lisp-mode.el
+++ b/lisp/emacs-lisp/lisp-mode.el
@@ -755,49 +755,104 @@ lisp-indent-function
(defun lisp-ppss (&optional pos)
"Return Parse-Partial-Sexp State at POS, defaulting to point.
-Like to `syntax-ppss' but includes the character address of the
-last complete sexp in the innermost containing list at position
+Like `syntax-ppss' but includes the character address of the last
+complete sexp in the innermost containing list at position
2 (counting from 0). This is important for lisp indentation."
(unless pos (setq pos (point)))
(let ((pss (syntax-ppss pos)))
(if (nth 9 pss)
- (parse-partial-sexp (car (last (nth 9 pss))) pos)
+ (let ((sexp-start (car (last (nth 9 pss)))))
+ (parse-partial-sexp sexp-start pos nil nil (syntax-ppss sexp-start)))
pss)))
+(cl-defstruct (lisp-indent-state
+ (:constructor nil)
+ (:constructor lisp-indent-initial-state
+ (&aux (ppss (lisp-ppss))
+ (last-syntax-point (point))
+ (last-depth (car ppss))
+ (indent-stack (make-list (1+ last-depth) nil)))))
+ indent-stack ; Cached indentation, per depth.
+ ppss
+ last-depth
+ last-syntax-point)
+
+(defun lisp-indent--next-line (state)
+ "Pass depth to update cache, or parse state for indentation."
+ (pcase-let (((cl-struct lisp-indent-state
+ indent-stack ppss last-depth last-syntax-point)
+ state))
+ ;; Parse this line so we can learn the state to indent the
+ ;; next line.
+ (while (let ((last-sexp (nth 2 ppss)))
+ (setq ppss (parse-partial-sexp
+ last-syntax-point (progn (end-of-line) (point))
+ nil nil ppss))
+ ;; Preserve last sexp of state (position 2) for
+ ;; `calculate-lisp-indent', if we're at the same depth.
+ (if (and (not (nth 2 ppss)) (= last-depth (car ppss)))
+ (setf (nth 2 ppss) last-sexp)
+ (setq last-sexp (nth 2 ppss)))
+ ;; Skip over newlines within strings.
+ (nth 3 ppss))
+ (let ((string-start (nth 8 ppss)))
+ (setq ppss (parse-partial-sexp (point) (point-max)
+ nil nil ppss 'syntax-table))
+ (setf (nth 2 ppss) string-start)) ; Finished a complete string.
+ (setq last-syntax-point (point)))
+ (setq last-syntax-point (point))
+ (let* ((depth (car ppss))
+ (depth-delta (- depth last-depth)))
+ (cond ((< depth-delta 0)
+ (setq indent-stack (nthcdr (- depth-delta) indent-stack)))
+ ((> depth-delta 0)
+ (setq indent-stack (nconc (make-list depth-delta nil)
+ indent-stack))))
+ (setq last-depth depth))
+ (prog1
+ (let (indent)
+ (cond ((= (forward-line 1) 1) nil)
+ ((car indent-stack))
+ ((integerp (setq indent (calculate-lisp-indent ppss)))
+ (setf (car indent-stack) indent))
+ ((consp indent) ; (COLUMN CONTAINING-SEXP-START)
+ (car indent))
+ ;; This only happens if we're in a string.
+ (t (error "This shouldn't happen"))))
+ (setf (lisp-indent-state-indent-stack state) indent-stack)
+ (setf (lisp-indent-state-last-depth state) last-depth)
+ (setf (lisp-indent-state-last-syntax-point state) last-syntax-point)
+ (setf (lisp-indent-state-ppss state) ppss))))
+
(defun lisp-indent-region (start end)
"Indent region as Lisp code, efficiently."
(save-excursion
(setq end (copy-marker end))
(goto-char start)
+ (beginning-of-line)
;; The default `indent-region-line-by-line' doesn't hold a running
;; parse state, which forces each indent call to reparse from the
;; beginning. That has O(n^2) complexity.
- (let* ((parse-state (lisp-ppss start))
- (last-syntax-point start)
+ (let* ((parse-state (lisp-indent-initial-state))
(pr (unless (minibufferp)
(make-progress-reporter "Indenting region..." (point) end))))
+ (let ((ppss (lisp-indent-state-ppss parse-state)))
+ (unless (or (and (bolp) (eolp)) (nth 3 ppss))
+ (lisp-indent-line (calculate-lisp-indent ppss))))
(while (< (point) end)
- (unless (and (bolp) (eolp))
- (lisp-indent-line parse-state))
- (forward-line 1)
- (let ((last-sexp (nth 2 parse-state)))
- (setq parse-state (parse-partial-sexp last-syntax-point (point)
- nil nil parse-state))
- ;; It's important to preserve last sexp location for
- ;; `calculate-lisp-indent'.
- (unless (nth 2 parse-state)
- (setf (nth 2 parse-state) last-sexp))
- (setq last-syntax-point (point)))
+ (let ((indent (lisp-indent--next-line parse-state)))
+ (unless (or (and (bolp) (eolp)) (not indent))
+ (lisp-indent-line indent)))
(and pr (progress-reporter-update pr (point))))
(and pr (progress-reporter-done pr))
(move-marker end nil))))
-(defun lisp-indent-line (&optional parse-state)
+(defun lisp-indent-line (&optional indent)
"Indent current line as Lisp code."
(interactive)
(let ((pos (- (point-max) (point)))
(indent (progn (beginning-of-line)
- (calculate-lisp-indent (or parse-state (lisp-ppss))))))
+ (or indent (calculate-lisp-indent (lisp-ppss))))))
(skip-chars-forward " \t")
(if (or (null indent) (looking-at "\\s<\\s<\\s<"))
;; Don't alter indentation of a ;;; comment line
@@ -1117,16 +1172,7 @@ indent-sexp
If optional arg ENDPOS is given, indent each line, stopping when
ENDPOS is encountered."
(interactive)
- (let* ((indent-stack (list nil))
- ;; Use `syntax-ppss' to get initial state so we don't get
- ;; confused by starting inside a string. We don't use
- ;; `syntax-ppss' in the loop, because this is measurably
- ;; slower when we're called on a long list.
- (state (syntax-ppss))
- (init-depth (car state))
- (next-depth init-depth)
- (last-depth init-depth)
- (last-syntax-point (point)))
+ (let* ((parse-state (lisp-indent-initial-state)))
;; We need a marker because we modify the buffer
;; text preceding endpos.
(setq endpos (copy-marker
@@ -1136,64 +1182,20 @@ indent-sexp
(save-excursion (forward-sexp 1) (point)))))
(save-excursion
(while (< (point) endpos)
- ;; Parse this line so we can learn the state to indent the
- ;; next line. Preserve element 2 of the state (last sexp) for
- ;; `calculate-lisp-indent'.
- (let ((last-sexp (nth 2 state)))
- (while (progn
- (setq state (parse-partial-sexp
- last-syntax-point (progn (end-of-line) (point))
- nil nil state))
- (setq last-sexp (or (nth 2 state) last-sexp))
- ;; Skip over newlines within strings.
- (nth 3 state))
- (setq state (parse-partial-sexp (point) (point-max)
- nil nil state 'syntax-table))
- (setq last-sexp (or (nth 2 state) last-sexp))
- (setq last-syntax-point (point)))
- (setf (nth 2 state) last-sexp))
- (setq next-depth (car state))
- ;; If the line contains a comment indent it now with
- ;; `indent-for-comment'.
- (when (nth 4 state)
- (indent-for-comment)
- (end-of-line))
- (setq last-syntax-point (point))
- (when (< next-depth init-depth)
- (setq indent-stack (nconc indent-stack
- (make-list (- init-depth next-depth) nil))
- last-depth (- last-depth next-depth)
- next-depth init-depth))
- ;; Now indent the next line according to what we learned from
- ;; parsing the previous one.
- (forward-line 1)
- (when (< (point) endpos)
- (let ((depth-delta (- next-depth last-depth)))
- (cond ((< depth-delta 0)
- (setq indent-stack (nthcdr (- depth-delta) indent-stack)))
- ((> depth-delta 0)
- (setq indent-stack (nconc (make-list depth-delta nil)
- indent-stack))))
- (setq last-depth next-depth))
+ (let ((indent (lisp-indent--next-line parse-state)))
+ ;; If the line contains a comment indent it now with
+ ;; `indent-for-comment'.
+ (when (nth 4 (lisp-indent-state-ppss parse-state))
+ (save-excursion
+ (goto-char (lisp-indent-state-last-syntax-point parse-state))
+ (indent-for-comment)
+ (setf (lisp-indent-state-last-syntax-point parse-state)
+ (line-end-position))))
;; But not if the line is blank, or just a comment (we
;; already called `indent-for-comment' above).
(skip-chars-forward " \t")
- (unless (or (eolp) (eq (char-syntax (char-after)) ?<))
- (indent-line-to
- (or (car indent-stack)
- ;; The state here is actually to the end of the
- ;; previous line, but that's fine for our purposes.
- ;; And parsing over the newline would only destroy
- ;; element 2 (last sexp position).
- (let ((val (calculate-lisp-indent state)))
- (cond ((integerp val)
- (setf (car indent-stack) val))
- ((consp val) ; (COLUMN CONTAINING-SEXP-START)
- (car val))
- ;; `calculate-lisp-indent' only returns nil
- ;; when we're in a string, but this won't
- ;; happen because we skip strings above.
- (t (error "This shouldn't happen!"))))))))))
+ (unless (or (eolp) (eq (char-syntax (char-after)) ?<) (not indent))
+ (indent-line-to indent)))))
(move-marker endpos nil)))
(defun indent-pp-sexp (&optional arg)
diff --git a/test/lisp/emacs-lisp/lisp-mode-tests.el b/test/lisp/emacs-lisp/lisp-mode-tests.el
index 27f0bb5ec1..1f78eb3010 100644
--- a/test/lisp/emacs-lisp/lisp-mode-tests.el
+++ b/test/lisp/emacs-lisp/lisp-mode-tests.el
@@ -21,10 +21,7 @@
(require 'cl-lib)
(require 'lisp-mode)
-(ert-deftest indent-sexp ()
- "Test basics of \\[indent-sexp]."
- (with-temp-buffer
- (insert "\
+(defconst lisp-mode-tests--correctly-indented-sexp "\
\(a
(prog1
(prog1
@@ -42,9 +39,14 @@
2) ; comment
;; comment
b)")
+
+(ert-deftest indent-sexp ()
+ "Test basics of \\[indent-sexp]."
+ (with-temp-buffer
+ (insert lisp-mode-tests--correctly-indented-sexp)
(goto-char (point-min))
(let ((indent-tabs-mode nil)
- (correct (buffer-string)))
+ (correct lisp-mode-tests--correctly-indented-sexp))
(dolist (mode '(fundamental-mode emacs-lisp-mode))
(funcall mode)
(indent-sexp)
@@ -97,5 +99,78 @@
(indent-sexp)
(should (equal (buffer-string) correct)))))
+(ert-deftest lisp-indent-region ()
+ "Test basics of `lisp-indent-region'."
+ (with-temp-buffer
+ (insert lisp-mode-tests--correctly-indented-sexp)
+ (goto-char (point-min))
+ (let ((indent-tabs-mode nil)
+ (correct lisp-mode-tests--correctly-indented-sexp))
+ (emacs-lisp-mode)
+ (indent-region (point-min) (point-max))
+ ;; Don't mess up correctly indented code.
+ (should (string= (buffer-string) correct))
+ ;; Correctly add indentation.
+ (save-excursion
+ (while (not (eobp))
+ (delete-horizontal-space)
+ (forward-line)))
+ (indent-region (point-min) (point-max))
+ (should (equal (buffer-string) correct))
+ ;; Correctly remove indentation.
+ (save-excursion
+ (let ((n 0))
+ (while (not (eobp))
+ (unless (looking-at "noindent\\|^[[:blank:]]*$")
+ (insert (make-string n ?\s)))
+ (cl-incf n)
+ (forward-line))))
+ (indent-region (point-min) (point-max))
+ (should (equal (buffer-string) correct)))))
+
+
+(ert-deftest lisp-indent-region-defun-with-docstring ()
+ "Test Bug#26619."
+ (with-temp-buffer
+ (insert "\
+\(defun test ()
+ \"This is a test.
+Test indentation in emacs-lisp-mode\"
+ (message \"Hi!\"))")
+ (let ((indent-tabs-mode nil)
+ (correct (buffer-string)))
+ (emacs-lisp-mode)
+ (indent-region (point-min) (point-max))
+ (should (equal (buffer-string) correct)))))
+
+(ert-deftest lisp-indent-region-open-paren ()
+ (with-temp-buffer
+ (insert "\
+\(with-eval-after-load 'foo
+ (setq bar `(
+ baz)))")
+ (let ((indent-tabs-mode nil)
+ (correct (buffer-string)))
+ (emacs-lisp-mode)
+ (indent-region (point-min) (point-max))
+ (should (equal (buffer-string) correct)))))
+
+(ert-deftest lisp-indent-region-in-sexp ()
+ (with-temp-buffer
+ (insert "\
+\(when t
+ (when t
+ (list 1 2 3)
+ 'etc)
+ (quote etc)
+ (quote etc))")
+ (let ((indent-tabs-mode nil)
+ (correct (buffer-string)))
+ (emacs-lisp-mode)
+ (search-backward "1")
+ (indent-region (point) (point-max))
+ (should (equal (buffer-string) correct)))))
+
+
(provide 'lisp-mode-tests)
;;; lisp-mode-tests.el ends here
--
2.11.1
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#26619
; Package
emacs
.
(Sat, 29 Apr 2017 12:02:02 GMT)
Full text and
rfc822 format available.
Message #43 received at 26619 <at> debbugs.gnu.org (full text, mbox):
npostavs <at> users.sourceforge.net writes:
> I think I got it this time.
Thanks - let's see... ;-)
Michael.
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#26619
; Package
emacs
.
(Fri, 05 May 2017 15:56:02 GMT)
Full text and
rfc822 format available.
Message #46 received at 26619 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
On Fri, Apr 28, 2017 at 10:19 PM <npostavs <at> users.sourceforge.net> wrote:
>
> I think I got it this time.
>
The indentation is still not working right.\
(Again, below screenshots show the indentation how it used to be; the light
grey boxes show where the indentation moved back to after applying that
last patch.)
Below code that you can use as test:
https://github.com/kaushalmodi/.emacs.d/blob/master/init.el
[image: image.png]
Indentation lost in forms like eval-when-compile and when
[image: image.png]
This is the worst.
https://github.com/kaushalmodi/.emacs.d/blob/master/setup-files/setup-editing.el
[image: image.png]
I see this same issue on emacs -Q with your final patch applied too:
[image: image.png]
This indentation issue is very difficult to miss.
--
Kaushal Modi
[Message part 2 (text/html, inline)]
[image.png (image/png, inline)]
[image.png (image/png, inline)]
[image.png (image/png, inline)]
[image.png (image/png, inline)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#26619
; Package
emacs
.
(Fri, 05 May 2017 22:42:02 GMT)
Full text and
rfc822 format available.
Message #49 received at 26619 <at> debbugs.gnu.org (full text, mbox):
Kaushal Modi <kaushal.modi <at> gmail.com> writes:
> The indentation is still not working right.\
>
> (Again, below screenshots show the indentation how it used to be; the
> light grey boxes show where the indentation moved back to after
> applying that last patch.)
I get the correct indentation for the code below with the patch, how are
you testing it (note that it modifies a dumped lisp file, so just
recompiling is not sufficient)?
(setq org-directory (let ((dir (concat user-home-directory
"org/"))) ; must end with /
(make-directory dir :parents)
dir))
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#26619
; Package
emacs
.
(Sat, 06 May 2017 01:59:02 GMT)
Full text and
rfc822 format available.
Message #52 received at 26619 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
On Fri, May 5, 2017 at 6:41 PM <npostavs <at> users.sourceforge.net> wrote:
> Kaushal Modi <kaushal.modi <at> gmail.com> writes:
>
> > The indentation is still not working right.\
> >
> > (Again, below screenshots show the indentation how it used to be; the
> > light grey boxes show where the indentation moved back to after
> > applying that last patch.)
>
> I get the correct indentation for the code below with the patch, how are
> you testing it (note that it modifies a dumped lisp file, so just
> recompiling is not sufficient)?
>
> (setq org-directory (let ((dir (concat user-home-directory
> "org/"))) ; must end with /
> (make-directory dir :parents)
> dir))
>
Sorry, you are right. This patch worked great! It even fixed the
indentation over the previous version for forms like cl-case, and fixes the
alignment as in the case of concat shown in the second screencap below.
(Before is left, after is right)
[image: image.png]
[image: image.png]
This looks great!
Thanks.
--
Kaushal Modi
[Message part 2 (text/html, inline)]
[image.png (image/png, inline)]
[image.png (image/png, inline)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#26619
; Package
emacs
.
(Sat, 06 May 2017 02:41:02 GMT)
Full text and
rfc822 format available.
Message #55 received at 26619 <at> debbugs.gnu.org (full text, mbox):
Kaushal Modi <kaushal.modi <at> gmail.com> writes:
> Sorry, you are right. This patch worked great! It even fixed the
> indentation over the previous version for forms like cl-case, and fixes the
> alignment as in the case of concat shown in the second screencap below.
Hmm, the concat alignment is a bit fishy, but I see there is some
inconsistency between indent-sexp and indent-region even in Emacs 25, so
I won't try to fix that now.
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#26619
; Package
emacs
.
(Mon, 08 May 2017 12:47:02 GMT)
Full text and
rfc822 format available.
Message #58 received at 26619 <at> debbugs.gnu.org (full text, mbox):
Kaushal Modi <kaushal.modi <at> gmail.com> writes:
> Sorry, you are right. This patch worked great!
I also did not experience any trouble with the newest patch (I didn't
firmly test it, but I used it all the time).
Thanks,
Michael.
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#26619
; Package
emacs
.
(Wed, 10 May 2017 00:57:01 GMT)
Full text and
rfc822 format available.
Message #61 received at 26619 <at> debbugs.gnu.org (full text, mbox):
tags 26619 fixed
close 26619
quit
Michael Heerdegen <michael_heerdegen <at> web.de> writes:
> Kaushal Modi <kaushal.modi <at> gmail.com> writes:
>
>> Sorry, you are right. This patch worked great!
>
> I also did not experience any trouble with the newest patch (I didn't
> firmly test it, but I used it all the time).
Alright, pushed to master [1: e7b6751c0a].
[1: e7b6751c0a]: 2017-05-09 20:50:19 -0400
Fix lisp-indent-region and indent-sexp (Bug#26619)
http://git.savannah.gnu.org/cgit/emacs.git/commit/?id=e7b6751c0a74f24c14cd207d57a4e1a95f409256
Added tag(s) fixed.
Request was from
npostavs <at> users.sourceforge.net
to
control <at> debbugs.gnu.org
.
(Wed, 10 May 2017 00:57:02 GMT)
Full text and
rfc822 format available.
bug closed, send any further explanations to
26619 <at> debbugs.gnu.org and Tino Calancha <tino.calancha <at> gmail.com>
Request was from
npostavs <at> users.sourceforge.net
to
control <at> debbugs.gnu.org
.
(Wed, 10 May 2017 00:57:02 GMT)
Full text and
rfc822 format available.
bug archived.
Request was from
Debbugs Internal Request <help-debbugs <at> gnu.org>
to
internal_control <at> debbugs.gnu.org
.
(Wed, 07 Jun 2017 11:24:04 GMT)
Full text and
rfc822 format available.
This bug report was last modified 8 years and 71 days ago.
Previous Next
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.