From: Noam Postavsky Date: Sun, 23 Apr 2017 14:43:05 +0000 (-0400) Subject: Fix lisp-indent-region and indent-sexp (Bug#26619) X-Git-Tag: emacs-26.0.90~521^2~424 X-Git-Url: http://git.eshelyaron.com/gitweb/?a=commitdiff_plain;h=e7b6751c0a74f24c14cd207d57a4e1a95f409256;p=emacs.git Fix lisp-indent-region and indent-sexp (Bug#26619) The new lisp-indent-region introduced in 2017-04-22 "Add new `lisp-indent-region' that doesn't reparse the code." is broken because it doesn't save the calculated indent amounts for already seen sexp depths. Fix this by unifying the indent-sexp and lisp-indent-region code. Furthermore, only preserve position 2 of the running parse when the depth doesn't change. * lisp/emacs-lisp/lisp-mode.el (lisp-ppss): Use an OLDSTATE that corresponds with the start point when calling parse-partial-sexp. (lisp-indent-state): New struct. (lisp-indent-calc-next): New function, extracted from indent-sexp. (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. --- diff --git a/lisp/emacs-lisp/lisp-mode.el b/lisp/emacs-lisp/lisp-mode.el index 7448864ff99..6287f27b139 100644 --- a/lisp/emacs-lisp/lisp-mode.el +++ b/lisp/emacs-lisp/lisp-mode.el @@ -754,49 +754,108 @@ function is `common-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)) + (ppss-point (point)) + (depth (car ppss)) + (stack (make-list (1+ depth) nil))))) + stack ;; Cached indentation, per depth. + ppss + depth + ppss-point) + +(defun lisp-indent-calc-next (state) + "Move to next line and return calculated indent for it. +STATE is updated by side effect, the first state should be +created by `lisp-indent-initial-state'. This function may move +by more than one line to cross a string literal." + (pcase-let (((cl-struct lisp-indent-state + (stack indent-stack) ppss depth ppss-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 + ppss-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)) (= 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 ppss-point (point))) + (setq ppss-point (point)) + (let* ((next-depth (car ppss)) + (depth-delta (- next-depth 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 depth next-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-stack state) indent-stack) + (setf (lisp-indent-state-depth state) depth) + (setf (lisp-indent-state-ppss-point state) ppss-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)))) - (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))) - (and pr (progress-reporter-update pr (point)))) + (let ((ppss (lisp-indent-state-ppss parse-state))) + (unless (or (and (bolp) (eolp)) (nth 3 ppss)) + (lisp-indent-line (calculate-lisp-indent ppss)))) + (let ((indent nil)) + (while (progn (setq indent (lisp-indent-calc-next parse-state)) + (< (point) end)) + (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 @@ -1116,16 +1175,7 @@ Lisp function does not specify a special indentation." 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 @@ -1135,64 +1185,20 @@ ENDPOS is encountered." (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-calc-next 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-ppss-point parse-state)) + (indent-for-comment) + (setf (lisp-indent-state-ppss-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 27f0bb5ec13..1f78eb30105 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 @@ noindent\" 3 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 @@ noindent\" 3 (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