* bug#23419: 25.0.93; double-click-time
2016-05-02 8:03 bug#23419: 25.0.93; double-click-time martin rudalics
@ 2020-08-15 3:49 ` Stefan Kangas
2020-08-15 7:01 ` Eli Zaretskii
0 siblings, 1 reply; 4+ messages in thread
From: Stefan Kangas @ 2020-08-15 3:49 UTC (permalink / raw)
To: martin rudalics; +Cc: 23419
[-- Attachment #1: Type: text/plain, Size: 1236 bytes --]
tags 23419 + patch
thanks
martin rudalics <rudalics@gmx.at> writes:
> The documentation of ‘double-click-time’ is as:
>
> Maximum time between mouse clicks to make a double-click.
> Measured in milliseconds. The value nil means disable double-click
> recognition; t means double-clicks have no time limit and are detected
> by position only.
>
> However, most uses of ‘double-click-time’ in the lisp directory silently
> assume that the value is a number which typically fails in ‘sit-for’s
> like, for example, in help.el's
>
> (not (sit-for (/ double-click-time 1000.0) t))
>
> when ‘double-click-time’ is either t or nil.
>
> Could someone out there who understands the semantics of these
> ‘sit-for’s please fix them. I can eventually do that myself but these
> semantics are yet pretty unclear to me.
I can confirm this. To reproduce the issue in help.el, use:
0. emacs -Q
1. (setq double-click-time nil)
2. C-h k mouse-1
In the attached patch I add a new function `mouse-double-click-time'
which always returns a number. This seems better than adding
specialized logic everywhere to handle the various cases.
Any comments?
Best regards,
Stefan Kangas
[-- Attachment #2: 0001-Fix-handling-double-click-time-nil-or-t-Bug-23419.patch --]
[-- Type: text/x-diff, Size: 6050 bytes --]
From 81d8fcd69fd04fed6fbf0e0e88b8df1bf3f4f603 Mon Sep 17 00:00:00 2001
From: Stefan Kangas <stefankangas@gmail.com>
Date: Sat, 15 Aug 2020 05:41:16 +0200
Subject: [PATCH] Fix handling double-click-time nil or t (Bug#23419)
* lisp/mouse.el (mouse-double-click-time): New function to always
return a number for `double-click-time'.
* lisp/emulation/viper-mous.el (viper-multiclick-timeout):
* lisp/foldout.el (foldout-mouse-swallow-events):
* lisp/help.el (help--read-key-sequence):
* lisp/org/org-mouse.el (org-mouse-show-context-menu): Use
'mouse-double-click-time' instead of 'double-click-time'.
* src/keyboard.c (syms_of_keyboard): Mention
'mouse-double-click-time' in doc string of 'double-click-time'.
* test/lisp/mouse-tests.el (mouse-test-mouse-double-click-time):
New test.
---
lisp/emulation/viper-mous.el | 4 ++--
lisp/foldout.el | 2 +-
lisp/help.el | 2 +-
lisp/mouse.el | 11 +++++++++++
lisp/org/org-mouse.el | 2 +-
src/keyboard.c | 5 ++++-
test/lisp/mouse-tests.el | 14 ++++++++++++++
7 files changed, 34 insertions(+), 6 deletions(-)
diff --git a/lisp/emulation/viper-mous.el b/lisp/emulation/viper-mous.el
index 6ecfec548c..02f4ff4597 100644
--- a/lisp/emulation/viper-mous.el
+++ b/lisp/emulation/viper-mous.el
@@ -66,9 +66,9 @@ viper-surrounding-word-function
;; time interval in millisecond within which successive clicks are
;; considered related
(defcustom viper-multiclick-timeout (if (viper-window-display-p)
- double-click-time
+ (mouse-double-click-time)
500)
- "Time interval in millisecond within which successive mouse clicks are
+ "Time interval in milliseconds within which successive mouse clicks are
considered related."
:type 'integer
:group 'viper-mouse)
diff --git a/lisp/foldout.el b/lisp/foldout.el
index 0d7a7a88a6..4a3a02b22d 100644
--- a/lisp/foldout.el
+++ b/lisp/foldout.el
@@ -486,7 +486,7 @@ foldout-mouse-swallow-events
"Swallow intervening mouse events so we only get the final click-count.
Signal an error if the final event isn't the same type as the first one."
(let ((initial-event-type (event-basic-type event)))
- (while (null (sit-for (/ double-click-time 1000.0) 'nodisplay))
+ (while (null (sit-for (/ (mouse-double-click-time) 1000.0) 'nodisplay))
(setq event (read-event)))
(or (eq initial-event-type (event-basic-type event))
(error "")))
diff --git a/lisp/help.el b/lisp/help.el
index b7d867eb70..6760c04019 100644
--- a/lisp/help.el
+++ b/lisp/help.el
@@ -766,7 +766,7 @@ help--read-key-sequence
(memq 'down last-modifiers)
;; After a click, see if a double click is on the way.
(and (memq 'click last-modifiers)
- (not (sit-for (/ double-click-time 1000.0) t))))
+ (not (sit-for (/ (mouse-double-click-time) 1000.0) t))))
(let* ((seq (read-key-sequence "\
Describe the following key, mouse click, or menu item: "
nil nil 'can-return-switch-frame))
diff --git a/lisp/mouse.el b/lisp/mouse.el
index a06ca2a56c..0a62211dd7 100644
--- a/lisp/mouse.el
+++ b/lisp/mouse.el
@@ -154,6 +154,17 @@ key-translation-map
(define-key key-translation-map [double-mouse-1]
#'mouse--click-1-maybe-follows-link)
+(defun mouse-double-click-time ()
+ "Return a number for `double-click-time'.
+In contrast to using the `double-click-time' variable directly,
+which could be set to nil or t, this function is guaranteed to
+always return a positive integer or zero."
+ (let ((ct double-click-time))
+ (cond ((eq ct t) 10000) ; arbitrary number useful for sit-for
+ ((eq ct nil) 0)
+ ((and (numberp ct) (> ct 0)) ct)
+ (t 0))))
+
\f
;; Provide a mode-specific menu on a mouse button.
diff --git a/lisp/org/org-mouse.el b/lisp/org/org-mouse.el
index 02798874d2..0cc8c82fdd 100644
--- a/lisp/org/org-mouse.el
+++ b/lisp/org/org-mouse.el
@@ -210,7 +210,7 @@ org-mouse-show-context-menu
(interactive "@e \nP")
(if (and (= (event-click-count event) 1)
(or (not mark-active)
- (sit-for (/ double-click-time 1000.0))))
+ (sit-for (/ (mouse-double-click-time) 1000.0))))
(progn
(select-window (posn-window (event-start event)))
(when (not (org-mouse-mark-active))
diff --git a/src/keyboard.c b/src/keyboard.c
index 5fa58abce1..1e957ee91e 100644
--- a/src/keyboard.c
+++ b/src/keyboard.c
@@ -11887,7 +11887,10 @@ syms_of_keyboard (void)
doc: /* Maximum time between mouse clicks to make a double-click.
Measured in milliseconds. The value nil means disable double-click
recognition; t means double-clicks have no time limit and are detected
-by position only. */);
+by position only.
+
+In Lisp, you might want to use `mouse-double-click-time' instead of
+reading the value of this variable directly. */);
Vdouble_click_time = make_fixnum (500);
DEFVAR_INT ("double-click-fuzz", double_click_fuzz,
diff --git a/test/lisp/mouse-tests.el b/test/lisp/mouse-tests.el
index d520da7af5..d7f84ba04d 100644
--- a/test/lisp/mouse-tests.el
+++ b/test/lisp/mouse-tests.el
@@ -25,6 +25,20 @@
;;; Code:
+(ert-deftest mouse-test-mouse-double-click-time ()
+ (let ((double-click-time 500))
+ (should (= (mouse-double-click-time) 500)))
+ (let ((double-click-time 0))
+ (should (= (mouse-double-click-time) 0)))
+ (let ((double-click-time -500))
+ (should (= (mouse-double-click-time) 0)))
+ (let ((double-click-time nil))
+ (should (= (mouse-double-click-time) 0)))
+ (let ((double-click-time t))
+ (should (numberp (mouse-double-click-time))))
+ (let ((double-click-time '(invalid)))
+ (should (= (mouse-double-click-time) 0))))
+
(ert-deftest bug23288-use-return-value ()
"If `mouse-on-link-p' returns a string, its first character is used."
(cl-letf ((unread-command-events '((down-mouse-1 nil 1) (mouse-1 nil 1)))
--
2.28.0
^ permalink raw reply related [flat|nested] 4+ messages in thread