Followup to bug#23510

[Stephen Berman]

I sent the post below last night to the bug#23510 thread on the bug
list, but it never arrived, and I resent it twice today from two
different email addresses, and I also sent a test message to the bug
thread, but none of these have arrived, although other posts have
appeared in the list since my attempts were sent.  I've also tried to
open a new bug, but it apparently has also failed.  I've even mailed Eli
Zaretskii personally but gotten no reply, though he's had numerous posts
appear since I mailed him (he may just have a long queue, but usually he
replies fairly promptly).  I don't know what the problem is, but now I'm
making another attempt, here on emacs-devel; if this succeeds, I'd
appreciate it if someone forwards it to bug#23510 on the bug list.

Thanks,
Steve Berman

On Wed, 11 May 2016 12:44:43 +0300 Eli Zaretskii <address@hidden> wrote:

>> From: Stephen Berman <address@hidden>
>> Cc: "address@hidden" <address@hidden>,
>> address@hidden
>> Date: Wed, 11 May 2016 11:25:24 +0200
>> 
>> What you say about the use of pre- and post-command-hook and moving
>> overlays may well be (perhaps the main) part of the problem here, but I
>> found out that there's an specific condition inducing it: I noticed that
>> the flickering only happens when global-hl-line-mode is enabled, but not
>> when hl-line-mode is enabled, and it turns out that the crucial
>> difference between them is that in the former,
>> global-hl-line-sticky-flag is nil by default, while in the latter,
>> hl-line-sticky-flag is t by default: when I set
>> global-hl-line-sticky-flag to t and then enable global-hl-line-mode, the
>> flickering no longer happens, and when I set hl-line-sticky-flag to nil
>> and then enable hl-line-mode, the flickering does happen.
>
> If you can suggest a patch that fixes that and doesn't have any
> adverse effects, please do.

Ok, I've given it a try; patch below.  Debugging showed that the
flickering is caused by calling delete-overlay on pre-command-hook, so I
eliminated the uses of this hook and replaced them with the new
functions hl-line-maybe-unhighlight and global-hl-line-maybe-unhighlight
added to post-command-hook that call delete-overlay under more specific
conditions.  According to my tests this does not lead to flickering and
highlights and unhighlights as before (but others should test the code,
in case I missed or messed up anything).

I also noticed that the definition of global-hl-line-mode fails to
remove global-hl-line-highlight from change-major-mode-hook when the
mode is disabled; I assume this was an oversite, so I rectified it.

If my fix is found acceptable, I assume it should go to master, since
the flickering problem was probably always there, so is not a regression
in emacs-25.  With that in mind, I took the opportunity to make
global-hl-line-mode automatically taken effect in all live windows when
global-hl-line-sticky-flag is non-nil (the new function
global-hl-line-highlight-all); in the current (and I assume original)
implementation, enabling global-hl-line-mode only highlights the current
line of the selected window, even when global-hl-line-sticky-flag is
non-nil (the highlighting appears in other windows when they become
selected).  This change seems consistent with the idea of Global-Hl-Line
mode, expressed in its doc string ("Toggle line highlighting in all
buffers").  I tested it with three windows in each of two frames (the
most that comfortably fit on my display), and the highlighting was as
fast as with two windows on one frame, but I suppose it's possible that
more windows on more frames might be slower.

Steve Berman

diff --git a/lisp/hl-line.el b/lisp/hl-line.el
index 25c8a08..642a8ba 100644
--- a/lisp/hl-line.el
+++ b/lisp/hl-line.el
@@ -126,6 +126,9 @@ hl-line-range-function
 
 This variable is expected to be made buffer-local by modes.")
 
+(defvar hl-line-overlay-buffer nil
+  "Most recently visited buffer in which Hl-Line mode is enabled.")
+
 ;;;###autoload
 (define-minor-mode hl-line-mode
   "Toggle highlighting of the current line (Hl-Line mode).
@@ -142,22 +145,21 @@ hl-line-mode
 
 When `hl-line-sticky-flag' is nil, Hl-Line mode highlights the
 line about point in the selected window only.  In this case, it
-uses the function `hl-line-unhighlight' on `pre-command-hook' in
+uses the function `hl-line-maybe-unhighlight' in
 addition to `hl-line-highlight' on `post-command-hook'."
   :group 'hl-line
   (if hl-line-mode
       (progn
         ;; In case `kill-all-local-variables' is called.
         (add-hook 'change-major-mode-hook #'hl-line-unhighlight nil t)
-        (if hl-line-sticky-flag
-            (remove-hook 'pre-command-hook #'hl-line-unhighlight t)
-          (add-hook 'pre-command-hook #'hl-line-unhighlight nil t))
         (hl-line-highlight)
-       (add-hook 'post-command-hook #'hl-line-highlight nil t))
+        (setq hl-line-overlay-buffer (current-buffer))
+       (add-hook 'post-command-hook #'hl-line-highlight nil t)
+        (add-hook 'post-command-hook #'hl-line-maybe-unhighlight))
     (remove-hook 'post-command-hook #'hl-line-highlight t)
     (hl-line-unhighlight)
     (remove-hook 'change-major-mode-hook #'hl-line-unhighlight t)
-    (remove-hook 'pre-command-hook #'hl-line-unhighlight t)))
+    (remove-hook 'post-command-hook #'hl-line-maybe-unhighlight t)))
 
 (defun hl-line-make-overlay ()
   (let ((ol (make-overlay (point) (point))))
@@ -181,6 +183,22 @@ hl-line-unhighlight
   (when hl-line-overlay
     (delete-overlay hl-line-overlay)))
 
+(defun hl-line-maybe-unhighlight ()
+  "Maybe deactivate the Hl-Line overlay on the current line.
+Specifically, when `hl-line-sticky-flag' is nil deactivate all
+such overlays in all buffers except the current one."
+  (let ((hlob hl-line-overlay-buffer)
+        (curbuf (current-buffer)))
+    (when (and (not hl-line-sticky-flag)
+               (not (eq curbuf hlob))
+               (not (minibufferp)))
+      (with-current-buffer hlob
+        (when (overlayp hl-line-overlay)
+          (delete-overlay hl-line-overlay))))
+    (when (and (overlayp hl-line-overlay)
+               (eq (overlay-buffer hl-line-overlay) curbuf))
+      (setq hl-line-overlay-buffer curbuf))))
+
 ;;;###autoload
 (define-minor-mode global-hl-line-mode
   "Toggle line highlighting in all buffers (Global Hl-Line mode).
@@ -189,25 +207,24 @@ global-hl-line-mode
 the mode if ARG is omitted or nil.
 
 If `global-hl-line-sticky-flag' is non-nil, Global Hl-Line mode
-highlights the line about the current buffer's point in all
+highlights the line about the current buffer's point in all live
 windows.
 
-Global-Hl-Line mode uses the functions `global-hl-line-unhighlight' and
-`global-hl-line-highlight' on `pre-command-hook' and `post-command-hook'."
+Global-Hl-Line mode uses the functions `global-hl-line-highlight'
+and `global-hl-line-maybe-unhighlight' on `post-command-hook'."
   :global t
   :group 'hl-line
   (if global-hl-line-mode
       (progn
         ;; In case `kill-all-local-variables' is called.
         (add-hook 'change-major-mode-hook #'global-hl-line-unhighlight)
-        (if global-hl-line-sticky-flag
-            (remove-hook 'pre-command-hook #'global-hl-line-unhighlight)
-          (add-hook 'pre-command-hook #'global-hl-line-unhighlight))
-        (global-hl-line-highlight)
-       (add-hook 'post-command-hook #'global-hl-line-highlight))
+        (global-hl-line-highlight-all)
+       (add-hook 'post-command-hook #'global-hl-line-highlight)
+        (add-hook 'post-command-hook #'global-hl-line-maybe-unhighlight))
     (global-hl-line-unhighlight-all)
-    (remove-hook 'pre-command-hook #'global-hl-line-unhighlight)
-    (remove-hook 'post-command-hook #'global-hl-line-highlight)))
+    (remove-hook 'post-command-hook #'global-hl-line-highlight)
+    (remove-hook 'change-major-mode-hook #'global-hl-line-unhighlight)
+    (remove-hook 'post-command-hook #'global-hl-line-maybe-unhighlight)))
 
 (defun global-hl-line-highlight ()
   "Highlight the current line in the current window."
@@ -222,11 +239,33 @@ global-hl-line-highlight
                     (selected-window)))
       (hl-line-move global-hl-line-overlay))))
 
+(defun global-hl-line-highlight-all ()
+  "Highlight the current line in all live windows."
+  (walk-windows (lambda (w)
+                  (with-current-buffer (window-buffer w)
+                    (global-hl-line-highlight)))
+                nil t))
+
 (defun global-hl-line-unhighlight ()
   "Deactivate the Global-Hl-Line overlay on the current line."
   (when global-hl-line-overlay
     (delete-overlay global-hl-line-overlay)))
 
+(defun global-hl-line-maybe-unhighlight ()
+  "Maybe deactivate the Global-Hl-Line overlay on the current line.
+Specifically, when `global-hl-line-sticky-flag' is nil deactivate
+all such overlays in all buffers except the current one."
+  (mapc (lambda (ov)
+         (let ((ovb (overlay-buffer ov)))
+            (when (and (not global-hl-line-sticky-flag)
+                       (bufferp ovb)
+                       (not (eq ovb (current-buffer)))
+                       (not (minibufferp)))
+               (with-current-buffer ovb
+                  (when (overlayp global-hl-line-overlay)
+                    (delete-overlay global-hl-line-overlay))))))
+        global-hl-line-overlays))
+
 (defun global-hl-line-unhighlight-all ()
   "Deactivate all Global-Hl-Line overlays."
   (mapc (lambda (ov)