From: npostavs@users.sourceforge.net
To: Michael Heerdegen <michael_heerdegen@web.de>
Cc: 26619@debbugs.gnu.org, Kaushal Modi <kaushal.modi@gmail.com>
Subject: bug#26619: 26.0.50; Wrong indentation in emacs-lisp-mode
Date: Fri, 28 Apr 2017 22:20:55 -0400 [thread overview]
Message-ID: <87efwcned4.fsf@users.sourceforge.net> (raw)
In-Reply-To: <87wpa62hlc.fsf@drachen> (Michael Heerdegen's message of "Thu, 27 Apr 2017 13:52:15 +0200")
[-- 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
next prev parent reply other threads:[~2017-04-29 2:20 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-04-23 7:17 bug#26619: 26.0.50; Wrong indentation in emacs-lisp-mode Tino Calancha
2017-04-23 14:51 ` npostavs
2017-04-24 21:03 ` Kaushal Modi
2017-04-26 3:05 ` Michael Heerdegen
2017-04-26 3:22 ` npostavs
2017-04-26 3:29 ` Michael Heerdegen
2017-04-26 3:53 ` npostavs
2017-04-26 4:00 ` Michael Heerdegen
2017-04-26 18:27 ` Kaushal Modi
2017-04-26 22:31 ` npostavs
2017-04-27 11:52 ` Michael Heerdegen
2017-04-29 2:20 ` npostavs [this message]
2017-04-29 12:00 ` Michael Heerdegen
2017-05-05 15:55 ` Kaushal Modi
2017-05-05 22:43 ` npostavs
2017-05-06 1:58 ` Kaushal Modi
2017-05-06 2:42 ` npostavs
2017-05-08 12:46 ` Michael Heerdegen
2017-05-10 0:57 ` npostavs
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87efwcned4.fsf@users.sourceforge.net \
--to=npostavs@users.sourceforge.net \
--cc=26619@debbugs.gnu.org \
--cc=kaushal.modi@gmail.com \
--cc=michael_heerdegen@web.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this external index
https://git.savannah.gnu.org/cgit/emacs.git
https://git.savannah.gnu.org/cgit/emacs/org-mode.git
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.