bug#24372: 25.1.50; After losing focus, cursor is hidden when moving point

[Philipp Stephani <p.stephani2@gmail.com>]

[-- Attachment #1.1: Type: text/plain, Size: 1368 bytes --]

Eli Zaretskii <eliz@gnu.org> schrieb am Sa., 10. Sep. 2016 um 09:21 Uhr:

> > From: Philipp Stephani <p.stephani2@gmail.com>
> > Date: Fri, 09 Sep 2016 17:18:12 +0000
> > Cc: 24372@debbugs.gnu.org
> >
> >  A simpler recipe that doesn't need explicit focus events is
> >
> >  emacs -Q -eval '(progn (setq blink-cursor-delay 0.0)
> (blink-cursor-suspend) (blink-cursor-check))'
> >
> >  and then start moving point.
> >
> > OK, I guess one issue is that setting blink-cursor-delay doesn't restart
> blink-cursor-idle-timer. (Similarly,
> > changing blink-cursor-interval doesn't restart blink-cursor-timer.)
> While obviously we can't fix that when using
> > setq, I'd suggest adding custom setters to the variables nevertheless.
>

I've attached a patch for this. It shouldn't be controversial because it
only reduces the possibility for surprises, but doesn't change any behavior.


> >
> > The direct cause of the issue seems to be that, when blink-cursor-delay
> is idle, after every command
> > blink-cursor-start is called immediately, which hides the cursor.
>
> Thanks.  Does the patch below fix the issue, without introducing any
> adverse side effects?
>
>
It does introduce the adverse side effect that now the first blink takes
one second (the sum of cursor-blink-delay and cursor-blink-interval). I've
attached another patch with the change I have in mind.

[-- Attachment #1.2: Type: text/html, Size: 2448 bytes --]

[-- Attachment #2: 0001-Avoid-hiding-the-blinking-cursor-too-fast.patch --]
[-- Type: application/octet-stream, Size: 2392 bytes --]

From 095920029806b1c599c9618818105e22e25321f5 Mon Sep 17 00:00:00 2001
From: Philipp Stephani <phst@google.com>
Date: Sun, 11 Sep 2016 11:07:18 +0200
Subject: [PATCH] Avoid hiding the blinking cursor too fast

If `blink-cursor-delay' is smaller than `blink-cursor-delay', the cursor
is hidden to quickly after a command; see Bug#24372.  This change uses
`blink-cursor-interval' as lower bound for `blink-cursor-delay'.

* lisp/frame.el (blink-cursor-check, blink-cursor-mode): Use
`blink-cursor-interval' as lower bound to `blink-cursor-delay'.
(blink-cursor-delay): Document changed behavior of
`blink-cursor-delay'.
---
 lisp/frame.el | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/lisp/frame.el b/lisp/frame.el
index cfd40bf..ebc5cc1 100644
--- a/lisp/frame.el
+++ b/lisp/frame.el
@@ -2027,7 +2027,11 @@ cursor
   :group 'frames)
 
 (defcustom blink-cursor-delay 0.5
-  "Seconds of idle time after which cursor starts to blink."
+  "Seconds of idle time after which cursor starts to blink.
+This is the interval between the end of an user action and the
+time when the cursor first becomes invisible due to blinking.  If
+this is shorter than `blink-cursor-interval', the value of
+`blink-cursor-interval' is used instead."
   :type 'number
   :group 'cursor)
 
@@ -2114,9 +2118,8 @@ blink-cursor-check
 	     (not blink-cursor-idle-timer))
     (remove-hook 'post-command-hook 'blink-cursor-check)
     (setq blink-cursor-idle-timer
-          (run-with-idle-timer blink-cursor-delay
-                               blink-cursor-delay
-                               'blink-cursor-start))))
+          (let ((delay (max blink-cursor-delay blink-cursor-interval)))
+            (run-with-idle-timer delay delay #'blink-cursor-start)))))
 
 (define-obsolete-variable-alias 'blink-cursor 'blink-cursor-mode "22.1")
 
@@ -2148,9 +2151,8 @@ blink-cursor-mode
     (add-hook 'focus-in-hook #'blink-cursor-check)
     (add-hook 'focus-out-hook #'blink-cursor-suspend)
     (setq blink-cursor-idle-timer
-          (run-with-idle-timer blink-cursor-delay
-                               blink-cursor-delay
-                               #'blink-cursor-start))))
+          (let ((delay (max blink-cursor-delay blink-cursor-interval)))
+            (run-with-idle-timer delay delay #'blink-cursor-start)))))
 
 \f
 ;; Frame maximization/fullscreen
-- 
2.9.0


[-- Attachment #3: 0001-Restart-blink-cursor-timers-on-interval-changes.patch --]
[-- Type: application/octet-stream, Size: 3899 bytes --]

From 06445b3e6fee4ab1c680ab4d9aedb75eddf85352 Mon Sep 17 00:00:00 2001
From: Philipp Stephani <phst@google.com>
Date: Sat, 10 Sep 2016 10:16:32 +0200
Subject: [PATCH] Restart blink cursor timers on interval changes

This prevents surprising behavior when timer interval customizations are
only applied whenever the timers happen to be restarted (see Bug#24372).

* lisp/frame.el (blink-cursor--start-idle-timer)
(blink-cursor--start-timer): New functions.
(blink-cursor-start, blink-cursor-check, blink-cursor-mode): Use
the new helper functions.
(blink-cursor-delay, blink-cursor-interval): Restart timers when
the value is changed.
---
 lisp/frame.el | 38 +++++++++++++++++++++++++-------------
 1 file changed, 25 insertions(+), 13 deletions(-)

diff --git a/lisp/frame.el b/lisp/frame.el
index cfd40bf..b32ba7a 100644
--- a/lisp/frame.el
+++ b/lisp/frame.el
@@ -2029,12 +2029,18 @@ cursor
 (defcustom blink-cursor-delay 0.5
   "Seconds of idle time after which cursor starts to blink."
   :type 'number
-  :group 'cursor)
+  :group 'cursor
+  :set (lambda (symbol value)
+         (set-default symbol value)
+         (when blink-cursor-idle-timer (blink-cursor--start-idle-timer))))
 
 (defcustom blink-cursor-interval 0.5
   "Length of cursor blink interval in seconds."
   :type 'number
-  :group 'cursor)
+  :group 'cursor
+  :set (lambda (symbol value)
+         (set-default symbol value)
+         (when blink-cursor-timer (blink-cursor--start-timer))))
 
 (defcustom blink-cursor-blinks 10
   "How many times to blink before using a solid cursor on NS, X, and MS-Windows.
@@ -2055,6 +2061,20 @@ blink-cursor-timer
 This timer calls `blink-cursor-timer-function' every
 `blink-cursor-interval' seconds.")
 
+(defun blink-cursor--start-idle-timer ()
+  "Start the `blink-cursor-idle-timer'."
+  (when blink-cursor-idle-timer (cancel-timer blink-cursor-idle-timer))
+  (setq blink-cursor-idle-timer
+        (run-with-idle-timer blink-cursor-delay blink-cursor-delay
+                             #'blink-cursor-start)))
+
+(defun blink-cursor--start-timer ()
+  "Start the `blink-cursor-timer'."
+  (when blink-cursor-timer (cancel-timer blink-cursor-timer))
+  (setq blink-cursor-timer
+        (run-with-timer blink-cursor-interval blink-cursor-interval
+                        #'blink-cursor-timer-function)))
+
 (defun blink-cursor-start ()
   "Timer function called from the timer `blink-cursor-idle-timer'.
 This starts the timer `blink-cursor-timer', which makes the cursor blink
@@ -2064,9 +2084,7 @@ blink-cursor-start
     ;; Set up the timer first, so that if this signals an error,
     ;; blink-cursor-end is not added to pre-command-hook.
     (setq blink-cursor-blinks-done 1)
-    (setq blink-cursor-timer
-	  (run-with-timer blink-cursor-interval blink-cursor-interval
-			  'blink-cursor-timer-function))
+    (blink-cursor--start-timer)
     (add-hook 'pre-command-hook 'blink-cursor-end)
     (internal-show-cursor nil nil)))
 
@@ -2113,10 +2131,7 @@ blink-cursor-check
   (when (and blink-cursor-mode
 	     (not blink-cursor-idle-timer))
     (remove-hook 'post-command-hook 'blink-cursor-check)
-    (setq blink-cursor-idle-timer
-          (run-with-idle-timer blink-cursor-delay
-                               blink-cursor-delay
-                               'blink-cursor-start))))
+    (blink-cursor--start-idle-timer)))
 
 (define-obsolete-variable-alias 'blink-cursor 'blink-cursor-mode "22.1")
 
@@ -2147,10 +2162,7 @@ blink-cursor-mode
   (when blink-cursor-mode
     (add-hook 'focus-in-hook #'blink-cursor-check)
     (add-hook 'focus-out-hook #'blink-cursor-suspend)
-    (setq blink-cursor-idle-timer
-          (run-with-idle-timer blink-cursor-delay
-                               blink-cursor-delay
-                               #'blink-cursor-start))))
+    (blink-cursor--start-idle-timer)))
 
 \f
 ;; Frame maximization/fullscreen
-- 
2.9.0