all messages for Emacs-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
From: npostavs@users.sourceforge.net
To: Thierry Volpiatto <thierry.volpiatto@gmail.com>
Cc: 25122@debbugs.gnu.org, Stefan Monnier <monnier@iro.umontreal.ca>,
	Boruch Baum <boruch_baum@gmx.com>
Subject: bug#25122: 24.5; function describe-variable hangs on large variables
Date: Mon, 13 Mar 2017 10:01:13 -0400	[thread overview]
Message-ID: <87innd6zti.fsf@users.sourceforge.net> (raw)
In-Reply-To: <87lgs97pg5.fsf@users.sourceforge.net> (npostavs@users.sourceforge.net's message of "Mon, 13 Mar 2017 00:47:38 -0400")

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

npostavs@users.sourceforge.net writes:

> npostavs@users.sourceforge.net writes:
>
>> Also, this doesn't really solve the performance problem, it just makes
>> it much less likely to occur, e.g., (pp (list load-history)) is still
>> slow.
>
> Okay, I think I found the real fix now:

Missed a corner case.


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

From b8dd372f65ce8979324c00b12a9ae767c1ffabda Mon Sep 17 00:00:00 2001
From: Noam Postavsky <npostavs@gmail.com>
Date: Sun, 12 Mar 2017 23:59:19 -0400
Subject: [PATCH v2] Don't reparse the sexp in indent-sexp (Bug#25122)

* lisp/emacs-lisp/lisp-mode.el (calculate-lisp-indent): Let
PARSE-START be a parse state that can be reused.
(indent-sexp): Pass the running parse state to calculate-lisp-indent
instead of the sexp beginning position.  Saving the
CONTAINING-SEXP-START returned by `calculate-lisp-indent' is no longer
needed.
* test/lisp/emacs-lisp/lisp-mode-tests.el (indent-sexp): Add blank
line to test case.
---
 lisp/emacs-lisp/lisp-mode.el            | 71 ++++++++++++++++++---------------
 test/lisp/emacs-lisp/lisp-mode-tests.el |  5 ++-
 2 files changed, 42 insertions(+), 34 deletions(-)

diff --git a/lisp/emacs-lisp/lisp-mode.el b/lisp/emacs-lisp/lisp-mode.el
index eb07c18b03..3fefb69066 100644
--- a/lisp/emacs-lisp/lisp-mode.el
+++ b/lisp/emacs-lisp/lisp-mode.el
@@ -781,6 +781,10 @@ calculate-lisp-indent
 If the value is nil, that means don't change the indentation
 because the line starts inside a string.
 
+PARSE-START may be a buffer position to start parsing from, or a
+parse state as returned by calling `parse-partial-sexp' up to the
+beginning of the current line.
+
 The value can also be a list of the form (COLUMN CONTAINING-SEXP-START).
 This means that following lines at the same level of indentation
 should not necessarily be indented the same as this line.
@@ -794,12 +798,14 @@ calculate-lisp-indent
           (desired-indent nil)
           (retry t)
           calculate-lisp-indent-last-sexp containing-sexp)
-      (if parse-start
-          (goto-char parse-start)
-          (beginning-of-defun))
-      ;; Find outermost containing sexp
-      (while (< (point) indent-point)
-        (setq state (parse-partial-sexp (point) indent-point 0)))
+      (cond ((or (markerp parse-start) (integerp parse-start))
+             (goto-char parse-start))
+            ((null parse-start) (beginning-of-defun))
+            (t (setq state parse-start)))
+      (unless state
+        ;; Find outermost containing sexp
+        (while (< (point) indent-point)
+          (setq state (parse-partial-sexp (point) indent-point 0))))
       ;; Find innermost containing sexp
       (while (and retry
 		  state
@@ -1070,11 +1076,6 @@ indent-sexp
 ENDPOS is encountered."
   (interactive)
   (let* ((indent-stack (list nil))
-         ;; If ENDPOS is non-nil, use beginning of defun as STARTING-POINT.
-         ;; If ENDPOS is nil, it is safe not to scan before point
-         ;; since every line we indent is more deeply nested than point is.
-         (starting-point (save-excursion (if endpos (beginning-of-defun))
-                                         (point)))
          ;; 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
@@ -1093,16 +1094,21 @@ indent-sexp
     (save-excursion
       (while (< (point) endpos)
         ;; Parse this line so we can learn the state to indent the
-        ;; next line.
-        (while (progn
-                 (setq state (parse-partial-sexp
-                              last-syntax-point (progn (end-of-line) (point))
-                              nil nil state))
-                 ;; Skip over newlines within strings.
-                 (nth 3 state))
-          (setq state (parse-partial-sexp (point) (point-max)
-                                          nil nil state 'syntax-table))
-          (setq last-syntax-point (point)))
+        ;; 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'.
@@ -1115,6 +1121,8 @@ indent-sexp
                                     (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)))
@@ -1124,28 +1132,25 @@ indent-sexp
                    (setq indent-stack (nconc (make-list depth-delta nil)
                                              indent-stack))))
             (setq last-depth next-depth))
-          ;; Now indent the next line according
-          ;; to what we learned from parsing the previous one.
-          (skip-chars-forward " \t")
           ;; 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)) ?<))
-            (let ((this-indent (car indent-stack)))
-              (when (listp this-indent)
-                (let ((val (calculate-lisp-indent
-                            (or (car this-indent) starting-point))))
-                  (setq
-                   this-indent
+            (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)
-                          (setf (car indent-stack) (cdr val))
                           (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!"))))))
-              (indent-line-to this-indent))))))))
+                         (t (error "This shouldn't happen!"))))))))))))
 
 (defun indent-pp-sexp (&optional arg)
   "Indent each line of the list starting just after point, or prettyprint it.
diff --git a/test/lisp/emacs-lisp/lisp-mode-tests.el b/test/lisp/emacs-lisp/lisp-mode-tests.el
index 2801f23df6..848dd7ca3e 100644
--- a/test/lisp/emacs-lisp/lisp-mode-tests.el
+++ b/test/lisp/emacs-lisp/lisp-mode-tests.el
@@ -31,6 +31,9 @@
          1
        2)
    2)
+ (fun arg1
+
+      arg2)
  (1
   \"string
 noindent\" (\"string2
@@ -58,7 +61,7 @@
         (save-excursion
           (let ((n 0))
             (while (not (eobp))
-              (unless (looking-at "noindent")
+              (unless (looking-at "noindent\\|^[[:blank:]]*$")
                 (insert (make-string n ?\s)))
               (cl-incf n)
               (forward-line))))
-- 
2.11.1


  reply	other threads:[~2017-03-13 14:01 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-12-06  2:21 bug#25122: 24.5; function describe-variable hangs on large variables Boruch Baum
2016-12-06  6:41 ` Thierry Volpiatto
2016-12-07  3:50   ` npostavs
2016-12-07  8:58     ` Thierry Volpiatto
2017-03-11  5:40       ` npostavs
2017-03-11 15:33         ` Stefan Monnier
2017-03-11 19:29           ` Thierry Volpiatto
2017-03-11 21:59             ` npostavs
2017-03-11 23:55               ` Drew Adams
2017-03-12  5:57               ` Thierry Volpiatto
2017-03-12 14:07                 ` Stefan Monnier
2017-03-12 14:15                 ` npostavs
2017-03-12 14:59                   ` Drew Adams
2017-03-12 16:29                   ` Stefan Monnier
2017-03-12 16:32                   ` npostavs
2017-03-13  4:47                     ` npostavs
2017-03-13 14:01                       ` npostavs [this message]
2017-03-16  2:54                         ` npostavs
2017-04-18  3:53                           ` npostavs
2017-04-22 18:25                             ` npostavs
2017-04-26  3:57                               ` Michael Heerdegen
2017-04-26 10:35                                 ` Michael Heerdegen
2017-03-11 19:34         ` Thierry Volpiatto
2017-03-12 16:07           ` npostavs
2017-03-11 15:21   ` Stefan Monnier
2017-03-11 15:35     ` npostavs
2017-03-11 19:26     ` Thierry Volpiatto

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=87innd6zti.fsf@users.sourceforge.net \
    --to=npostavs@users.sourceforge.net \
    --cc=25122@debbugs.gnu.org \
    --cc=boruch_baum@gmx.com \
    --cc=monnier@iro.umontreal.ca \
    --cc=thierry.volpiatto@gmail.com \
    /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.