Eli Zaretskii <
eliz@gnu.org> schrieb am So., 11. Sep. 2016 um 18:23 Uhr:
> From: Philipp Stephani <p.stephani2@gmail.com>
> Date: Sun, 11 Sep 2016 09:15:49 +0000
> Cc: 24372@debbugs.gnu.org
>
> > 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.
Using the maximum of blink-cursor-delay and blink-cursor-interval
effectively removes the user's ability of controlling the delay before
the cursor starts blinking when Emacs becomes idle, doesn't it?
Yes. I tried to figure out what the intention of blink-cursor-delay was, but couldn't find anything (the commit where it was introduced doesn't provide any motivation). I don't think that blink-cursor-delay < blink-cursor-interval ever makes sense. The other way round (delay > interval) is useful if you want to keep the cursor visible for a bit longer after a command.
If
so, I don't think this could qualify as not changing any behavior.
My comment is about the other patch, the one that fixes the customization options.
Plus, if the user sets the interval to a very small value, we have the
same problem again.
True, but so far that hasn't happened. Also it seems less surprising: if you increase the frequency, the cursor blinks faster, as expected.
How about a variant of this below? It uses a fixed limitation from
below on the delay, but only for the first blink. (The value 0.2 was
found by experimentation, not sure if we need to add yet another
defcustom for that.)
I don't think we should introduce magic numbers or further customization options. (TBH, I already doubt the usefulness of blink-cursor-delay, but that's already there.)
> 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).
Right.
> I've attached another patch with the change I have in mind.
This has a disadvantage of creating a new timer object each time,
which I think we'd like to avoid: too much consing. (Also, don't you
need to set the timer variable to nil when the timer is disabled?)
I don't understand - the patch doesn't create any additional timers, it only changes the initial delay of the idle-timer.
I'd prefer something along the lines of the first idea, the patch
below or some variant of it. It is simpler.
diff --git a/lisp/frame.el b/lisp/frame.el
index cfd40bf..4540172 100644
--- a/lisp/frame.el
+++ b/lisp/frame.el
@@ -2114,7 +2114,7 @@ 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
+ (run-with-idle-timer (max 0.2 blink-cursor-delay)
blink-cursor-delay
'blink-cursor-start))))
@@ -2148,7 +2148,7 @@ 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
+ (run-with-idle-timer (max 0.2 blink-cursor-delay)
blink-cursor-delay
#'blink-cursor-start))))
My patch is identical, except is uses blink-cursor-interval as lower bound.