From: Phil Sainty Date: Mon, 21 Oct 2019 10:52:29 +0000 (+1300) Subject: Defer triggering `so-long' until the buffer is displayed X-Git-Tag: emacs-27.0.90~626^2^2~5 X-Git-Url: http://git.eshelyaron.com/gitweb/?a=commitdiff_plain;h=e9dca2b5aa43116980321259682ad0c68fd0f937;p=emacs.git Defer triggering `so-long' until the buffer is displayed * lisp/so-long.el (so-long-invisible-buffer-function): New user option. (so-long--set-auto-mode): Use so-long-invisible-buffer-function. (so-long-deferred): New function/value for so-long-invisible-buffer-function. (so-long, so-long--disable): Support for so-long-deferred. * test/lisp/so-long-tests/autoload-longlines-mode-tests.el * test/lisp/so-long-tests/autoload-major-mode-tests.el * test/lisp/so-long-tests/autoload-minor-mode-tests.el * test/lisp/so-long-tests/so-long-tests.el: Support for so-long-deferred. Pre-existing tests have been updated to ensure the buffer is already displayed in cases where a call to `normal-mode' is the (potential) trigger for `so-long'. --- diff --git a/lisp/so-long.el b/lisp/so-long.el index 8d9b0dc0627..f7dfc8a79ca 100644 --- a/lisp/so-long.el +++ b/lisp/so-long.el @@ -162,6 +162,23 @@ ;; this option can also be configured to inhibit so-long entirely in this ;; scenario, or to not treat a file-local mode as a special case at all. +;; * Buffers which are not displayed in a window +;; --------------------------------------------- +;; When a file with long lines is visited and the buffer is not displayed right +;; away, it may be that it is not intended to be displayed at all, and that it +;; has instead been visited for behind-the-scenes processing by some library. +;; Invisible buffers are less likely to cause performance issues, and it also +;; might be surprising to the other library if such a buffer were manipulated by +;; `so-long' (which might in turn lead to confusing errors for the user); so in +;; these situations the `so-long-invisible-buffer-function' value is called +;; instead. By default this arranges for `so-long' to be invoked on the buffer +;; if and when it is displayed, but not otherwise. +;; +;; This 'deferred call' is actually the most common scenario -- even when a +;; visited file is displayed "right away", it is normal for the buffer to be +;; invisible when `global-so-long-mode' processes it, and the gap between +;; "arranging to call" and "calling" `so-long' is simply extremely brief. + ;; * Inhibiting and disabling minor modes ;; -------------------------------------- ;; Certain minor modes cause significant performance issues in the presence of @@ -345,6 +362,7 @@ ;; - New user option `so-long-variable-overrides'. ;; - New user option `so-long-skip-leading-comments'. ;; - New user option `so-long-file-local-mode-function'. +;; - New user option `so-long-invisible-buffer-function'. ;; - New user option `so-long-predicate'. ;; - New variable and function `so-long-function'. ;; - New variable and function `so-long-revert-function'. @@ -487,6 +505,37 @@ files would prevent Emacs from handling them correctly." :package-version '(so-long . "1.0") :group 'so-long) +(defcustom so-long-invisible-buffer-function #'so-long-deferred + "Function called in place of `so-long' when the buffer is not displayed. + +This affects the behaviour of `global-so-long-mode'. + +We treat invisible buffers differently from displayed buffers because, in +cases where a library is using a buffer for behind-the-scenes processing, +it might be surprising if that buffer were unexpectedly manipulated by +`so-long' (which might in turn lead to confusing errors for the user). +Invisible buffers are less likely to cause performance issues related to +long lines, so this differentiation is generally satisfactory. + +The default value `so-long-deferred' prevents `global-so-long-mode' from +triggering `so-long' for any given buffer until such time as the buffer is +displayed in a window. + +\(Note that buffers are normally invisible at this point -- when `find-file' +is used, the buffer is not displayed in a window until a short time after +`global-so-long-mode' has seen it.) + +The value nil or `so-long' means that `so-long' will be called directly; in +which case it may be problematic for `so-long-variable-overrides' to enable +`buffer-read-only', or for `so-long-action' to be set to `so-long-mode'. +This is because the buffer may not be intended to be displayed at all, and +the mentioned options might interfere with some intended processing." + :type '(radio (const so-long-deferred) + (const :tag "nil: Call so-long as normal" nil) + (function :tag "Custom function")) + :package-version '(so-long . "1.0") + :group 'so-long) + (defcustom so-long-predicate 'so-long-detected-long-line-p "Function, called after `set-auto-mode' to decide whether action is needed. @@ -1486,7 +1535,17 @@ major mode is a member (or derivative of a member) of `so-long-target-modes'. (or (eq so-long-target-modes t) (apply #'derived-mode-p so-long-target-modes)) (setq so-long-detected-p (funcall so-long-predicate)) - (so-long))) + ;; `so-long' should be called; but only if and when the buffer is + ;; displayed in a window. Long lines in invisible buffers are generally + ;; not problematic, whereas it might cause problems if an invisible + ;; buffer being used for behind-the-scenes processing is manipulated + ;; unexpectedly. The default `so-long-invisible-buffer-function' value + ;; is `so-long-deferred', which arranges to call `so-long' as soon as + ;; the buffer is displayed. + (if (or (get-buffer-window (current-buffer) t) + (not so-long-invisible-buffer-function)) + (so-long) + (funcall so-long-invisible-buffer-function)))) (defun so-long--hack-one-local-variable (orig-fun var val) ;; Advice, enabled with: @@ -1530,6 +1589,14 @@ These local variables will thus not vanish on setting a major mode." ;; VAR is not the 'mode' pseudo-variable. (funcall orig-fun var val))) +(defun so-long-deferred () + "Arrange to call `so-long' if the current buffer is displayed in a window." + ;; The first time that a window-configuration change results in the buffer + ;; being displayed in a window, `so-long' will be called (with the window + ;; selected and the buffer set as current). Because `so-long' removes this + ;; buffer-local hook value, it triggers once at most. + (add-hook 'window-configuration-change-hook #'so-long nil :local)) + ;;;###autoload (defun so-long (&optional action) "Invoke `so-long-action' and run `so-long-hook'. @@ -1547,6 +1614,8 @@ argument, select the action to use interactively." (completing-read "Action (none): " (mapcar #'car so-long-action-alist) nil :require-match))))) + ;; Ensure that `so-long-deferred' only triggers `so-long' once (at most). + (remove-hook 'window-configuration-change-hook #'so-long :local) (unless so-long--calling (let ((so-long--calling t)) (so-long--ensure-enabled) @@ -1693,6 +1762,12 @@ or call the function `global-so-long-mode'.") (defun so-long-unload-function () "Handler for `unload-feature'." (global-so-long-mode 0) + ;; Remove buffer-local `window-configuration-change-hook' values set by + ;; `so-long-deferred'. + (dolist (buf (buffer-list)) + (with-current-buffer buf + (remove-hook 'window-configuration-change-hook #'so-long :local))) + ;; Return nil. Refer to `unload-feature'. nil) diff --git a/test/lisp/so-long-tests/autoload-longlines-mode-tests.el b/test/lisp/so-long-tests/autoload-longlines-mode-tests.el index 5a57e049fb5..c94aeaef24b 100644 --- a/test/lisp/so-long-tests/autoload-longlines-mode-tests.el +++ b/test/lisp/so-long-tests/autoload-longlines-mode-tests.el @@ -40,6 +40,7 @@ (ert-deftest so-long-tests-autoload-longlines-mode () "File-local -*- so-long-action: longlines-mode; eval: (so-long) -*-" (with-temp-buffer + (display-buffer (current-buffer)) (so-long-tests-remember) (insert "-*- so-long-action: longlines-mode; eval: (so-long) -*-\n") (put 'so-long-action 'safe-local-variable #'symbolp) diff --git a/test/lisp/so-long-tests/autoload-major-mode-tests.el b/test/lisp/so-long-tests/autoload-major-mode-tests.el index d82cb59750c..a8f6f9e7b32 100644 --- a/test/lisp/so-long-tests/autoload-major-mode-tests.el +++ b/test/lisp/so-long-tests/autoload-major-mode-tests.el @@ -38,6 +38,7 @@ (ert-deftest so-long-tests-autoload-major-mode () "File-local -*- so-long -*-" (with-temp-buffer + (display-buffer (current-buffer)) (so-long-tests-remember) (insert "-*- so-long -*-\n") (normal-mode) diff --git a/test/lisp/so-long-tests/autoload-minor-mode-tests.el b/test/lisp/so-long-tests/autoload-minor-mode-tests.el index 67f1903c09c..600a35de0a9 100644 --- a/test/lisp/so-long-tests/autoload-minor-mode-tests.el +++ b/test/lisp/so-long-tests/autoload-minor-mode-tests.el @@ -39,6 +39,7 @@ (ert-deftest so-long-tests-autoload-minor-mode () "File-local -*- so-long-action: so-long-minor-mode; eval: (so-long) -*-" (with-temp-buffer + (display-buffer (current-buffer)) (so-long-tests-remember) (insert "-*- so-long-action: so-long-minor-mode; eval: (so-long) -*-\n") (put 'so-long-action 'safe-local-variable #'symbolp) diff --git a/test/lisp/so-long-tests/so-long-tests.el b/test/lisp/so-long-tests/so-long-tests.el index b1e0cb90d00..99af5e91ba0 100644 --- a/test/lisp/so-long-tests/so-long-tests.el +++ b/test/lisp/so-long-tests/so-long-tests.el @@ -29,13 +29,19 @@ ;; (We could consistently use the latter, but the mixture of approaches ;; means that we're testing more things.) -;; Running the tests with "make lisp/so-long-tests" is like: +;; Running manually: ;; -;; HOME=/nonexistent EMACSLOADPATH= LC_ALL=C \ -;; EMACS_TEST_DIRECTORY=/home/phil/emacs/trunk/repository/test \ +;; for test in lisp/so-long-tests/*-tests.el; do make ${test%.el}; done \ +;; 2>&1 | egrep -v '^(Loading|Source file|make|Changed to so-long-mode)' +;; +;; Which is equivalent to: +;; +;; for test in lisp/so-long-tests/*-tests.el; do \ +;; HOME=/nonexistent EMACSLOADPATH= LC_ALL=C EMACS_TEST_DIRECTORY=. \ ;; "../src/emacs" --no-init-file --no-site-file --no-site-lisp \ -;; -L ":." -l ert -l lisp/so-long-tests.el --batch --eval \ -;; '(ert-run-tests-batch-and-exit (quote (not (tag :unstable))))' +;; -L ":." -l ert -l "$test" --batch --eval \ +;; '(ert-run-tests-batch-and-exit (quote (not (tag :unstable))))'; \ +;; done 2>&1 | egrep -v '^(Loading|Source file|Changed to so-long-mode)' ;; ;; See also `ert-run-tests-batch-and-exit'. @@ -58,6 +64,7 @@ (ert-deftest so-long-tests-threshold-under () "Under line length threshold." (with-temp-buffer + (display-buffer (current-buffer)) (insert "#!emacs\n") (insert (make-string (1- so-long-threshold) ?x)) (normal-mode) @@ -66,6 +73,7 @@ (ert-deftest so-long-tests-threshold-at () "At line length threshold." (with-temp-buffer + (display-buffer (current-buffer)) (insert "#!emacs\n") (insert (make-string (1- so-long-threshold) ?x)) (normal-mode) @@ -74,6 +82,7 @@ (ert-deftest so-long-tests-threshold-over () "Over line length threshold." (with-temp-buffer + (display-buffer (current-buffer)) (insert "#!emacs\n") (normal-mode) (so-long-tests-remember) @@ -85,12 +94,14 @@ "Skip leading shebang, whitespace, and comments." ;; Long comment, no newline. (with-temp-buffer + (display-buffer (current-buffer)) (insert "#!emacs\n") (insert (make-string (1+ so-long-threshold) ?\;)) (normal-mode) (should (eq major-mode 'emacs-lisp-mode))) ;; Long comment, with newline. (with-temp-buffer + (display-buffer (current-buffer)) (insert "#!emacs\n") (insert (make-string (1+ so-long-threshold) ?\;)) (insert "\n") @@ -98,6 +109,7 @@ (should (eq major-mode 'emacs-lisp-mode))) ;; Long comment, with short text following. (with-temp-buffer + (display-buffer (current-buffer)) (insert "#!emacs\n") (insert (make-string (1+ so-long-threshold) ?\;)) (insert "\n") @@ -106,6 +118,7 @@ (should (eq major-mode 'emacs-lisp-mode))) ;; Long comment, with long text following. (with-temp-buffer + (display-buffer (current-buffer)) (insert "#!emacs\n") (insert (make-string (1+ so-long-threshold) ?\;)) (insert "\n") @@ -116,6 +129,7 @@ (ert-deftest so-long-tests-max-lines () "Give up after `so-long-max-lines'." (with-temp-buffer + (display-buffer (current-buffer)) (insert "#!emacs\n") ;; Insert exactly `so-long-max-lines' non-comment lines, followed ;; by a long line. @@ -139,10 +153,91 @@ (normal-mode) (should (eq major-mode 'so-long-mode)))))) +(ert-deftest so-long-tests-invisible-buffer-function () + "Call `so-long-invisible-buffer-function' in invisible buffers." + ;; Visible buffer. + (with-temp-buffer + (display-buffer (current-buffer)) + (insert "#!emacs\n") + (normal-mode) + (so-long-tests-remember) + (insert (make-string (1+ so-long-threshold) ?x)) + (normal-mode) + (so-long-tests-assert-and-revert 'so-long-mode)) + ;; Invisible buffer. + (with-temp-buffer + (insert "#!emacs\n") + (normal-mode) + (so-long-tests-remember) + (insert (make-string (1+ so-long-threshold) ?x)) + (normal-mode) + (should (eq major-mode 'emacs-lisp-mode)) + (should (eq nil (get-buffer-window))) + ;; Displaying the buffer should invoke `so-long'. + (display-buffer (current-buffer)) + (should (window-live-p (get-buffer-window))) + (unless (version< emacs-version "27") + ;; From Emacs 27 the `display-buffer' call is insufficient. + ;; The various 'window change functions' are now invoked by the + ;; redisplay, and redisplay does nothing at all in batch mode, + ;; so we cannot test under this revised behaviour. Refer to: + ;; https://lists.gnu.org/archive/html/emacs-devel/2019-10/msg00971.html + ;; For interactive (non-batch) test runs, calling `redisplay' + ;; does do the trick; so do that first. + (redisplay) + (when noninteractive + ;; In batch mode we need to cheat, and just pretend that + ;; `redisplay' triggered `window-configuration-change-hook'. + ;; This means the test is not as useful, but it still covers + ;; part of the process, and so it's better than nothing. + ;; + ;; Also test `so-long--active', in case a future version of + ;; Emacs adds the framework necessary to make `redisplay' work + ;; in batch mode. + (unless (eq so-long--active t) + (run-window-configuration-change-hook)))) + (so-long-tests-assert-and-revert 'so-long-mode)) + ;; `so-long-invisible-buffer-function' is `nil'. + (with-temp-buffer + (insert "#!emacs\n") + (normal-mode) + (so-long-tests-remember) + (insert (make-string (1+ so-long-threshold) ?x)) + (let ((so-long-invisible-buffer-function nil)) + (normal-mode)) + (so-long-tests-assert-and-revert 'so-long-mode)) + ;; `so-long-invisible-buffer-function' is `so-long'. + (with-temp-buffer + (insert "#!emacs\n") + (normal-mode) + (so-long-tests-remember) + (insert (make-string (1+ so-long-threshold) ?x)) + (let ((so-long-invisible-buffer-function #'so-long)) + (normal-mode)) + (so-long-tests-assert-and-revert 'so-long-mode)) + ;; `so-long-invisible-buffer-function' is `ignore'. + (with-temp-buffer + (insert "#!emacs\n") + (normal-mode) + (so-long-tests-remember) + (insert (make-string (1+ so-long-threshold) ?x)) + (let ((so-long-invisible-buffer-function #'ignore)) + (normal-mode)) + (should (eq major-mode 'emacs-lisp-mode)) + (display-buffer (current-buffer)) + (unless (version< emacs-version "27") + ;; See the "Invisible buffer" case earlier in this function. + (redisplay) + (when noninteractive + (unless (eq so-long--active t) + (run-window-configuration-change-hook)))) + (should (eq major-mode 'emacs-lisp-mode)))) + (ert-deftest so-long-tests-actions () "Test each of the standard actions." (dolist (action (mapcar #'car so-long-action-alist)) (with-temp-buffer + (display-buffer (current-buffer)) (insert "#!emacs\n") (normal-mode) (so-long-tests-remember) @@ -210,6 +305,7 @@ "Targeted major modes." ;; Test the `so-long-target-modes' user option. (with-temp-buffer + (display-buffer (current-buffer)) (insert "#!emacs\n") (insert (make-string (1+ so-long-threshold) ?x)) ;; Nil target modes. @@ -233,6 +329,7 @@ "Custom predicate function." ;; Test the `so-long-predicate' user option. (with-temp-buffer + (display-buffer (current-buffer)) (insert "#!emacs\n") ;; Always false. (let ((so-long-predicate #'ignore)) @@ -257,6 +354,7 @@ ;; valid for the file-locals to be on the second line after the shebang, ;; but with the *.el filename we no longer need the shebang. (with-temp-buffer + (display-buffer (current-buffer)) (setq buffer-file-name (expand-file-name "so-long-tests-data.el")) (insert ";; -*- so-long-action:so-long-minor-mode; -*-\n") (put 'so-long-action 'safe-local-variable #'symbolp) @@ -275,6 +373,7 @@ (normal-mode) (so-long-tests-remember)) (with-temp-buffer + (display-buffer (current-buffer)) (setq buffer-file-name (concat (make-temp-name "so-long-tests-") ".el")) (insert ";; -*- so-long-action:so-long-minor-mode; eval:(so-long) -*-\n") (put 'so-long-action 'safe-local-variable #'symbolp) @@ -314,6 +413,7 @@ ;; Downgrade the action from major mode to minor mode. (setq-default so-long-file-local-mode-function 'so-long-mode-downgrade) (with-temp-buffer + (display-buffer (current-buffer)) (insert ,prop-line) (insert (make-string (1+ so-long-threshold) ?x)) (insert ,local-vars) @@ -322,6 +422,7 @@ ;; Do not treat the file-local mode specially. (setq-default so-long-file-local-mode-function nil) (with-temp-buffer + (display-buffer (current-buffer)) (insert ,prop-line) (insert (make-string (1+ so-long-threshold) ?x)) (insert ,local-vars) @@ -331,6 +432,7 @@ (setq-default so-long-file-local-mode-function #'so-long-tests-file-local-mode-function) (with-temp-buffer + (display-buffer (current-buffer)) (insert ,prop-line) (insert (make-string (1+ so-long-threshold) ?x)) (insert ,local-vars) @@ -371,6 +473,7 @@ ;; Do nothing at all when a file-local mode is used. (setq-default so-long-file-local-mode-function 'so-long-inhibit) (with-temp-buffer + (display-buffer (current-buffer)) ;; Remember the new-buffer state. The other cases will ;; validate the 'reverted' state against this. (so-long-tests-remember) @@ -382,6 +485,7 @@ ;; Downgrade from major mode to minor mode. (setq-default so-long-file-local-mode-function 'so-long-mode-downgrade) (with-temp-buffer + (display-buffer (current-buffer)) (insert ,prop-line) (insert (make-string (1+ so-long-threshold) ?x)) (insert ,local-vars) @@ -390,6 +494,7 @@ ;; Do not treat the file-local mode specially. (setq-default so-long-file-local-mode-function nil) (with-temp-buffer + (display-buffer (current-buffer)) (insert ,prop-line) (insert (make-string (1+ so-long-threshold) ?x)) (insert ,local-vars) @@ -399,6 +504,7 @@ (setq-default so-long-file-local-mode-function #'so-long-tests-file-local-mode-function) (with-temp-buffer + (display-buffer (current-buffer)) (insert ,prop-line) (insert (make-string (1+ so-long-threshold) ?x)) (insert ,local-vars)