bug#45946: [PATCH] Re: bug#45946: 28.0.50; hl-line-sticky-flag not working

[Gabriel do Nascimento Ribeiro <gabriel376@hotmail.com>]

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

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Gabriel do Nascimento Ribeiro <gabriel376@hotmail.com>
>> Date: Wed, 20 Jan 2021 22:07:33 -0300
>> 
>> Here is a proposal of patch for this bug. It seems to work well in all
>> cases I have tested, but maybe I am missing some edge case. Suggestions
>> are welcome.
>
> Thanks.
>
> While at that, would it be possible to fix some code there that I at
> least consider strange?  E.g., it calls delete-overlay, but doesn't
> assign nil to the variable, so the overlayp predicate still returns
> non-nil for the resulting invalid overlay.  Also, it puts 2 functions
> on the post-command-hook, but it looks like the code subtly depends on
> the order of their execution (should hl-line-highlight run before or
> after hl-line-maybe-unhighlight?).

Hi Eli,

Thanks for your suggestions. Please find attached a new patch that uses
a single post-command-hook function and some other minor changes. The
original issue is fixed.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Use-single-post-command-hook-on-hl-line.patch --]
[-- Type: text/x-diff, Size: 7133 bytes --]

From 980504403e41164fef79806781eff84e05760463 Mon Sep 17 00:00:00 2001
From: Gabriel do Nascimento Ribeiro <gabriel.nascimento@nubank.com.br>
Date: Fri, 22 Jan 2021 23:05:17 -0300
Subject: [PATCH] Use single post-command-hook on hl-line

---
 lisp/hl-line.el | 58 +++++++++++++++++++++----------------------------
 1 file changed, 25 insertions(+), 33 deletions(-)

diff --git a/lisp/hl-line.el b/lisp/hl-line.el
index 73870f9579..82952e934b 100644
--- a/lisp/hl-line.el
+++ b/lisp/hl-line.el
@@ -45,11 +45,7 @@
 ;; An overlay is used.  In the non-sticky cases, this overlay is
 ;; active only on the selected window.  A hook is added to
 ;; `post-command-hook' to activate the overlay and move it to the line
-;; about point.  To get the non-sticky behavior, `hl-line-unhighlight'
-;; is added to `pre-command-hook' as well.  This function deactivates
-;; the overlay unconditionally in case the command changes the
-;; selected window.  (It does so rather than keeping track of changes
-;; in the selected window).
+;; about point.
 
 ;; You could make variable `global-hl-line-mode' buffer-local and set
 ;; it to nil to avoid highlighting specific buffers, when the global
@@ -91,9 +87,9 @@ hl-line-face
 	 (set symbol value)
 	 (dolist (buffer (buffer-list))
 	   (with-current-buffer buffer
-	     (when hl-line-overlay
+	     (when (overlayp hl-line-overlay)
 	       (overlay-put hl-line-overlay 'face hl-line-face))))
-	 (when global-hl-line-overlay
+	 (when (overlayp global-hl-line-overlay)
 	   (overlay-put global-hl-line-overlay 'face hl-line-face))))
 
 (defcustom hl-line-sticky-flag t
@@ -141,9 +137,7 @@ hl-line-mode
 `hl-line-highlight' on `post-command-hook' in this case.
 
 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-maybe-unhighlight' in
-addition to `hl-line-highlight' on `post-command-hook'."
+line about point in the selected window only."
   :group 'hl-line
   (if hl-line-mode
       (progn
@@ -151,12 +145,10 @@ hl-line-mode
         (add-hook 'change-major-mode-hook #'hl-line-unhighlight nil t)
         (hl-line-highlight)
         (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 nil t))
+	(add-hook 'post-command-hook #'hl-line-highlight nil t))
     (remove-hook 'post-command-hook #'hl-line-highlight t)
     (hl-line-unhighlight)
-    (remove-hook 'change-major-mode-hook #'hl-line-unhighlight t)
-    (remove-hook 'post-command-hook #'hl-line-maybe-unhighlight t)))
+    (remove-hook 'change-major-mode-hook #'hl-line-unhighlight t)))
 
 (defun hl-line-make-overlay ()
   (let ((ol (make-overlay (point) (point))))
@@ -168,17 +160,19 @@ hl-line-highlight
   "Activate the Hl-Line overlay on the current line."
   (if hl-line-mode	; Might be changed outside the mode function.
       (progn
-        (unless hl-line-overlay
+        (unless (overlayp hl-line-overlay)
           (setq hl-line-overlay (hl-line-make-overlay))) ; To be moved.
         (overlay-put hl-line-overlay
                      'window (unless hl-line-sticky-flag (selected-window)))
-	(hl-line-move hl-line-overlay))
+	(hl-line-move hl-line-overlay)
+        (hl-line-maybe-unhighlight))
     (hl-line-unhighlight)))
 
 (defun hl-line-unhighlight ()
   "Deactivate the Hl-Line overlay on the current line."
-  (when hl-line-overlay
-    (delete-overlay hl-line-overlay)))
+  (when (overlayp hl-line-overlay)
+    (delete-overlay hl-line-overlay)
+    (setq hl-line-overlay nil)))
 
 (defun hl-line-maybe-unhighlight ()
   "Maybe deactivate the Hl-Line overlay on the current line.
@@ -191,8 +185,7 @@ hl-line-maybe-unhighlight
                (not (eq curbuf hlob))
                (not (minibufferp)))
       (with-current-buffer hlob
-        (when (overlayp hl-line-overlay)
-          (delete-overlay hl-line-overlay))))
+        (hl-line-unhighlight)))
     (when (and (overlayp hl-line-overlay)
                (eq (overlay-buffer hl-line-overlay) curbuf))
       (setq hl-line-overlay-buffer curbuf))))
@@ -205,8 +198,8 @@ global-hl-line-mode
 highlights the line about the current buffer's point in all live
 windows.
 
-Global-Hl-Line mode uses the functions `global-hl-line-highlight'
-and `global-hl-line-maybe-unhighlight' on `post-command-hook'."
+Global-Hl-Line mode uses the function `global-hl-line-highlight'
+on `post-command-hook'."
   :global t
   :group 'hl-line
   (if global-hl-line-mode
@@ -214,25 +207,24 @@ global-hl-line-mode
         ;; In case `kill-all-local-variables' is called.
         (add-hook 'change-major-mode-hook #'global-hl-line-unhighlight)
         (global-hl-line-highlight-all)
-	(add-hook 'post-command-hook #'global-hl-line-highlight)
-        (add-hook 'post-command-hook #'global-hl-line-maybe-unhighlight))
+	(add-hook 'post-command-hook #'global-hl-line-highlight))
     (global-hl-line-unhighlight-all)
     (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)))
+    (remove-hook 'change-major-mode-hook #'global-hl-line-unhighlight)))
 
 (defun global-hl-line-highlight ()
   "Highlight the current line in the current window."
   (when global-hl-line-mode	; Might be changed outside the mode function.
     (unless (window-minibuffer-p)
-      (unless global-hl-line-overlay
+      (unless (overlayp global-hl-line-overlay)
         (setq global-hl-line-overlay (hl-line-make-overlay))) ; To be moved.
       (unless (member global-hl-line-overlay global-hl-line-overlays)
 	(push global-hl-line-overlay global-hl-line-overlays))
       (overlay-put global-hl-line-overlay 'window
 		   (unless global-hl-line-sticky-flag
 		     (selected-window)))
-      (hl-line-move global-hl-line-overlay))))
+      (hl-line-move global-hl-line-overlay)
+      (global-hl-line-maybe-unhighlight))))
 
 (defun global-hl-line-highlight-all ()
   "Highlight the current line in all live windows."
@@ -243,8 +235,9 @@ global-hl-line-highlight-all
 
 (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)))
+  (when (overlayp global-hl-line-overlay)
+    (delete-overlay global-hl-line-overlay)
+    (setq global-hl-line-overlay nil)))
 
 (defun global-hl-line-maybe-unhighlight ()
   "Maybe deactivate the Global-Hl-Line overlay on the current line.
@@ -256,9 +249,8 @@ global-hl-line-maybe-unhighlight
                        (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))))))
+	      (with-current-buffer ovb
+                (global-hl-line-unhighlight)))))
         global-hl-line-overlays))
 
 (defun global-hl-line-unhighlight-all ()
-- 
2.27.0