bug#26619: 26.0.50; Wrong indentation in emacs-lisp-mode

bug#26619: 26.0.50; Wrong indentation in emacs-lisp-mode

[Tino Calancha]

X-Debbugs-CC: npostavs@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

bug#26619: 26.0.50; Wrong indentation in emacs-lisp-mode

[npostavs]

[-- Attachment #1: Type: text/plain, Size: 559 bytes --]

tags 26619 patch
quit

Tino Calancha <tino.calancha@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:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: patch --]
[-- Type: text/x-diff, Size: 10681 bytes --]

From bb640d2f1570b38d98950fb0c374f66ef0988bd2 Mon Sep 17 00:00:00 2001
From: Noam Postavsky <npostavs@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

bug#26619: 26.0.50; Wrong indentation in emacs-lisp-mode

[Kaushal Modi]

[-- Attachment #1: Type: text/plain, Size: 508 bytes --]

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

[-- Attachment #2: Type: text/html, Size: 946 bytes --]

bug#26619: 26.0.50; Wrong indentation in emacs-lisp-mode

[Michael Heerdegen]

Kaushal Modi <kaushal.modi@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.

bug#26619: 26.0.50; Wrong indentation in emacs-lisp-mode

[npostavs]

[-- Attachment #1: Type: text/plain, Size: 672 bytes --]

Kaushal Modi <kaushal.modi@gmail.com> writes:

>
> But indentation is still broken in cases like these:
>
> (with-eval-after-load 'foo
> (setq bar `(
> baz)))
>

Michael Heerdegen <michael_heerdegen@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.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: patch, part 2 --]
[-- Type: text/x-diff, Size: 3532 bytes --]

From 101fa466c3e5303458dc6abd2d24ef55d508d9cd Mon Sep 17 00:00:00 2001
From: Noam Postavsky <npostavs@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

bug#26619: 26.0.50; Wrong indentation in emacs-lisp-mode

[Michael Heerdegen]

npostavs@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.

bug#26619: 26.0.50; Wrong indentation in emacs-lisp-mode

[npostavs]

[-- Attachment #1: Type: text/plain, Size: 258 bytes --]

Michael Heerdegen <michael_heerdegen@web.de> writes:

> npostavs@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.


[-- Attachment #2: cumulative patch --]
[-- Type: text/plain, Size: 9317 bytes --]

diff --git c/lisp/emacs-lisp/lisp-mode.el w/lisp/emacs-lisp/lisp-mode.el
index fa931e76ad..12fd53140c 100644
--- c/lisp/emacs-lisp/lisp-mode.el
+++ w/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,31 +803,45 @@ 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)
-                                                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))))
       (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 +1161,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.
@@ -1139,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))
@@ -1149,51 +1191,33 @@ 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))
-        (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)

bug#26619: 26.0.50; Wrong indentation in emacs-lisp-mode

[Michael Heerdegen]

npostavs@users.sourceforge.net writes:

> Sure.

Thanks.  I'll have a look.


Michael.

bug#26619: 26.0.50; Wrong indentation in emacs-lisp-mode

[Kaushal Modi]

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

On Tue, Apr 25, 2017 at 11:52 PM <npostavs@users.sourceforge.net> wrote:

> Michael Heerdegen <michael_heerdegen@web.de> writes:
>
> > npostavs@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

[-- Attachment #1.2: Type: text/html, Size: 1532 bytes --]

[-- Attachment #2: pasted1 --]
[-- Type: image/png, Size: 235331 bytes --]

bug#26619: 26.0.50; Wrong indentation in emacs-lisp-mode

[npostavs]

[-- Attachment #1: Type: text/plain, Size: 302 bytes --]

Kaushal Modi <kaushal.modi@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.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: patch --]
[-- Type: text/x-diff, Size: 1832 bytes --]

---   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))))

[-- Attachment #3: cumulative patch --]
[-- Type: text/plain, Size: 8755 bytes --]

---   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,31 +803,40 @@ 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)
-                                                nil nil parse-state))
+        (let ((oldstate parse-state)
+              (target-point (point)))
+          (while
+              (progn
+                (setq parse-state (parse-partial-sexp last-syntax-point target-point
+                                                      (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'.
-          (unless (nth 2 parse-state)
-            (setf (nth 2 parse-state) last-sexp))
-          (setq last-syntax-point (point)))
+          ;; `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))))
       (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 +1156,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.
@@ -1139,7 +1175,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))
@@ -1149,51 +1186,33 @@ 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))
-        (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)

bug#26619: 26.0.50; Wrong indentation in emacs-lisp-mode

[Michael Heerdegen]

npostavs@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.

bug#26619: 26.0.50; Wrong indentation in emacs-lisp-mode

[npostavs]

[-- Attachment #1: Type: text/plain, Size: 30 bytes --]


I think I got it this time.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: patch --]
[-- Type: text/x-diff, Size: 14992 bytes --]

From 4b50ab92dddba5178ed5ea16a93ee5b53fec8f02 Mon Sep 17 00:00:00 2001
From: Noam Postavsky <npostavs@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

bug#26619: 26.0.50; Wrong indentation in emacs-lisp-mode

[Michael Heerdegen]

npostavs@users.sourceforge.net writes:

> I think I got it this time.

Thanks - let's see... ;-)


Michael.

bug#26619: 26.0.50; Wrong indentation in emacs-lisp-mode

[Kaushal Modi]

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

On Fri, Apr 28, 2017 at 10:19 PM <npostavs@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

[-- Attachment #1.2: Type: text/html, Size: 1885 bytes --]

[-- Attachment #2: image.png --]
[-- Type: image/png, Size: 64196 bytes --]

[-- Attachment #3: image.png --]
[-- Type: image/png, Size: 38391 bytes --]

[-- Attachment #4: image.png --]
[-- Type: image/png, Size: 181823 bytes --]

[-- Attachment #5: image.png --]
[-- Type: image/png, Size: 95119 bytes --]

bug#26619: 26.0.50; Wrong indentation in emacs-lisp-mode

[npostavs]

Kaushal Modi <kaushal.modi@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))

bug#26619: 26.0.50; Wrong indentation in emacs-lisp-mode

[Kaushal Modi]

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

On Fri, May 5, 2017 at 6:41 PM <npostavs@users.sourceforge.net> wrote:

> Kaushal Modi <kaushal.modi@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

[-- Attachment #1.2: Type: text/html, Size: 1928 bytes --]

[-- Attachment #2: image.png --]
[-- Type: image/png, Size: 214086 bytes --]

[-- Attachment #3: image.png --]
[-- Type: image/png, Size: 226916 bytes --]

bug#26619: 26.0.50; Wrong indentation in emacs-lisp-mode

[npostavs]

Kaushal Modi <kaushal.modi@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.

bug#26619: 26.0.50; Wrong indentation in emacs-lisp-mode

[Michael Heerdegen]

Kaushal Modi <kaushal.modi@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.

bug#26619: 26.0.50; Wrong indentation in emacs-lisp-mode

[npostavs]

tags 26619 fixed
close 26619 
quit

Michael Heerdegen <michael_heerdegen@web.de> writes:

> Kaushal Modi <kaushal.modi@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