bug#28658: 27.0.50; [PATCH] double/triple clicking in xterm-mouse-mode doesn't respect mouse position

bug#28658: 27.0.50; [PATCH] double/triple clicking in xterm-mouse-mode doesn't respect mouse position

[Alex]

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

1. emacs -Q -nw
2. M-x xterm-mouse-mode RET
3. Click once, then quickly move the cursor to a different word and click again.
4. Notice that the 2nd click was registered as a double click even
though the cursor moved.
5. Click once, and quickly perform step 3 again. Notice that the 3rd
click was registered as a triple click even though the cursor moved.

I've attached a patch that fixes this behaviour. It would be nice if it
worked with pixel positions rather than character positions, but I'm not
sure how to do that in a terminal Emacs.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: click --]
[-- Type: text/x-diff, Size: 1760 bytes --]

From 77590f427ba962f5efc3f07d2e4dcadb4ec1237d Mon Sep 17 00:00:00 2001
From: Alexander Gramiak <agrambot@gmail.com>
Date: Sat, 30 Sep 2017 15:47:56 -0600
Subject: [PATCH] Increase xterm click count only with unchanged point

* lisp/xt-mouse.el (xterm-mouse-event): Save the last click's position
and check it against the current click's position.
---
 lisp/xt-mouse.el | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/lisp/xt-mouse.el b/lisp/xt-mouse.el
index 772a72d5c5..dc202e78f6 100644
--- a/lisp/xt-mouse.el
+++ b/lisp/xt-mouse.el
@@ -278,6 +278,8 @@ xterm-mouse-event
                (last-name (symbol-name last-type))
                (last-time (nth 1 last-click))
                (click-count (nth 2 last-click))
+               (last-x (nth 3 last-click))
+               (last-y (nth 4 last-click))
                (this-time (float-time))
                (name (symbol-name type)))
           (cond
@@ -290,12 +292,14 @@ xterm-mouse-event
               (xterm-mouse--set-click-count event click-count)))
            ((not last-time) nil)
            ((and (> double-click-time (* 1000 (- this-time last-time)))
+                 (eq x last-x)
+                 (eq y last-y)
                  (equal last-name (replace-match "" t t name)))
             (setq click-count (1+ click-count))
             (xterm-mouse--set-click-count event click-count))
            (t (setq click-count 1)))
           (set-terminal-parameter nil 'xterm-mouse-last-click
-                                  (list type this-time click-count)))
+                                  (list type this-time click-count x y)))
 
         (set-terminal-parameter nil 'xterm-mouse-x x)
         (set-terminal-parameter nil 'xterm-mouse-y y)
-- 
2.14.1

bug#28658: 27.0.50; [PATCH] double/triple clicking in xterm-mouse-mode doesn't respect mouse position

[Alex]

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

I also found another issue, but it doesn't really warrant a new report.
If you double click right after entering xterm-mouse-mode, then you'll
see:

xterm-mouse-translate-1: Wrong type argument: number-or-marker-p, nil

This is because click-count isn't set to 1 when `xterm-mouse-last-click'
is first set in `xterm-mouse-event'.

Here's a patch:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: init click --]
[-- Type: text/x-diff, Size: 1147 bytes --]

From 6bc82587b2c7554e93b603d0c5835f287673a7c5 Mon Sep 17 00:00:00 2001
From: Alexander Gramiak <agrambot@gmail.com>
Date: Sat, 30 Sep 2017 21:38:10 -0600
Subject: [PATCH] Set xterm click count to 1 even with no last click

* lisp/xt-mouse.el (xterm-mouse-event): Move the check for the last
click so that click-count is initialized properly. (Bug#28658)
---
 lisp/xt-mouse.el | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lisp/xt-mouse.el b/lisp/xt-mouse.el
index dc202e78f6..cbbeec82fa 100644
--- a/lisp/xt-mouse.el
+++ b/lisp/xt-mouse.el
@@ -290,8 +290,8 @@ xterm-mouse-event
                        (string-match "down-" last-name)
                        (equal name (replace-match "" t t last-name)))
               (xterm-mouse--set-click-count event click-count)))
-           ((not last-time) nil)
-           ((and (> double-click-time (* 1000 (- this-time last-time)))
+           ((and last-time
+                 (> double-click-time (* 1000 (- this-time last-time)))
                  (eq x last-x)
                  (eq y last-y)
                  (equal last-name (replace-match "" t t name)))
-- 
2.14.2


[-- Attachment #3: Type: text/plain, Size: 93 bytes --]


P.S. Why is the error prefixed by `xterm-mouse-translate-1' instead of
`xterm-mouse-event'?

bug#28658: 27.0.50; [PATCH] double/triple clicking in xterm-mouse-mode doesn't respect mouse position

[martin rudalics]

+           ((and last-time
+                 (> double-click-time (* 1000 (- this-time last-time)))

Note that here and elsewhere coders assume that ‘double-click-time’ is a
number.  However, it may be also nil or t and we should fix those
instances eventually (see Bug#23419).  In new or changed code, at least.

Thanks, martin

bug#28658: 27.0.50; [PATCH] double/triple clicking in xterm-mouse-mode doesn't respect mouse position

[Alex]

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

martin rudalics <rudalics@gmx.at> writes:

> +           ((and last-time
> +                 (> double-click-time (* 1000 (- this-time last-time)))
>
> Note that here and elsewhere coders assume that ‘double-click-time’ is a
> number.  However, it may be also nil or t and we should fix those
> instances eventually (see Bug#23419).  In new or changed code, at least.

Sure, here's a third patch to deal with that:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: boolean --]
[-- Type: text/x-diff, Size: 1034 bytes --]

From efb7f09f585d6652221f5e58e9bc635e6100a588 Mon Sep 17 00:00:00 2001
From: Alexander Gramiak <agrambot@gmail.com>
Date: Sun, 1 Oct 2017 12:21:19 -0600
Subject: [PATCH] * lisp/xt-mouse.el (xterm-mouse-event): Handle boolean
 double-click-time

---
 lisp/xt-mouse.el | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/lisp/xt-mouse.el b/lisp/xt-mouse.el
index cbbeec82fa..aa5aa0f7f5 100644
--- a/lisp/xt-mouse.el
+++ b/lisp/xt-mouse.el
@@ -291,7 +291,9 @@ xterm-mouse-event
                        (equal name (replace-match "" t t last-name)))
               (xterm-mouse--set-click-count event click-count)))
            ((and last-time
-                 (> double-click-time (* 1000 (- this-time last-time)))
+                 double-click-time
+                 (or (eq double-click-time t)
+                     (> double-click-time (* 1000 (- this-time last-time))))
                  (eq x last-x)
                  (eq y last-y)
                  (equal last-name (replace-match "" t t name)))
-- 
2.14.2

bug#28658: 27.0.50; [PATCH] double/triple clicking in xterm-mouse-mode doesn't respect mouse position

[Eli Zaretskii]

> From: Alex <agrambot@gmail.com>
> Date: Sun, 01 Oct 2017 12:30:23 -0600
> Cc: 28658@debbugs.gnu.org
> 
> martin rudalics <rudalics@gmx.at> writes:
> 
> > +           ((and last-time
> > +                 (> double-click-time (* 1000 (- this-time last-time)))
> >
> > Note that here and elsewhere coders assume that ‘double-click-time’ is a
> > number.  However, it may be also nil or t and we should fix those
> > instances eventually (see Bug#23419).  In new or changed code, at least.
> 
> Sure, here's a third patch to deal with that:

Pushed to emacs-26, thanks.

bug#28658: 27.0.50; [PATCH] double/triple clicking in xterm-mouse-mode doesn't respect mouse position

[Eli Zaretskii]

> From: Alex <agrambot@gmail.com>
> Date: Sat, 30 Sep 2017 21:56:58 -0600
> 
> I also found another issue, but it doesn't really warrant a new report.
> If you double click right after entering xterm-mouse-mode, then you'll
> see:
> 
> xterm-mouse-translate-1: Wrong type argument: number-or-marker-p, nil
> 
> This is because click-count isn't set to 1 when `xterm-mouse-last-click'
> is first set in `xterm-mouse-event'.
> 
> Here's a patch:

Thanks, pushed to the release branch.

> P.S. Why is the error prefixed by `xterm-mouse-translate-1' instead of
> `xterm-mouse-event'?

Can you show the full backtrace?

bug#28658: 27.0.50; [PATCH] double/triple clicking in xterm-mouse-mode doesn't respect mouse position

[Alex]

Eli Zaretskii <eliz@gnu.org> writes:

>> P.S. Why is the error prefixed by `xterm-mouse-translate-1' instead of
>> `xterm-mouse-event'?
>
> Can you show the full backtrace?

  Debugger entered--Lisp error: (wrong-type-argument number-or-marker-p nil)
    xterm-mouse-event(1006)
    xterm-mouse-translate-1(1006)
    xterm-mouse-translate-extended(nil)

So the backtrace acknowledges xterm-mouse-event is the culprit, but
without `debug-on-error' set the error message blames
xterm-mouse-translate-1.

If I don't use the bytecode versions of the procedures, though, then the
error message becomes:

  setq: Wrong type argument: number-or-marker-p, nil

which is better, but I believe it should message:

  1+: Wrong type argument: number-or-marker-p, nil

Which is what the debugger shows if I don't use the bytecode versions:

  Debugger entered--Lisp error: (wrong-type-argument number-or-marker-p nil)
    1+(nil)
    (setq click-count (1+ click-count))
    ...
    xterm-mouse-event(1006)
    (let* ((event (xterm-mouse-event extension)) (ev-command (nth 0 event)) (ev-data (nth 1 event)) (ev-where (nth 1 ev-$
    (save-excursion (let* ((event (xterm-mouse-event extension)) (ev-command (nth 0 event)) (ev-data (nth 1 event)) (ev-$
    xterm-mouse-translate-1(1006)
    xterm-mouse-translate-extended(nil)

bug#28658: 27.0.50; [PATCH] double/triple clicking in xterm-mouse-mode doesn't respect mouse position

[Eli Zaretskii]

> From: Alex <agrambot@gmail.com>
> Cc: 28658@debbugs.gnu.org
> Date: Thu, 05 Oct 2017 18:03:31 -0600
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >> P.S. Why is the error prefixed by `xterm-mouse-translate-1' instead of
> >> `xterm-mouse-event'?
> >
> > Can you show the full backtrace?
> 
>   Debugger entered--Lisp error: (wrong-type-argument number-or-marker-p nil)
>     xterm-mouse-event(1006)
>     xterm-mouse-translate-1(1006)
>     xterm-mouse-translate-extended(nil)
> 
> So the backtrace acknowledges xterm-mouse-event is the culprit, but
> without `debug-on-error' set the error message blames
> xterm-mouse-translate-1.
> 
> If I don't use the bytecode versions of the procedures, though, then the
> error message becomes:
> 
>   setq: Wrong type argument: number-or-marker-p, nil

So this is something related to the byte compiler, I presume.  Did you
look at the byte code?

bug#28658: 27.0.50; [PATCH] double/triple clicking in xterm-mouse-mode doesn't respect mouse position

[Alex]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Alex <agrambot@gmail.com>
>> Cc: 28658@debbugs.gnu.org
>> Date: Thu, 05 Oct 2017 18:03:31 -0600
>> 
>>   Debugger entered--Lisp error: (wrong-type-argument number-or-marker-p nil)
>>     xterm-mouse-event(1006)
>>     xterm-mouse-translate-1(1006)
>>     xterm-mouse-translate-extended(nil)
>> 
>> So the backtrace acknowledges xterm-mouse-event is the culprit, but
>> without `debug-on-error' set the error message blames
>> xterm-mouse-translate-1.
>> 
>> If I don't use the bytecode versions of the procedures, though, then the
>> error message becomes:
>> 
>>   setq: Wrong type argument: number-or-marker-p, nil
>
> So this is something related to the byte compiler, I presume.

For the most part, yeah. Though without the byte compiler the error
message is prefixed by `setq' instead of `1+' as it should be (and is
when debugging).

> Did you look at the byte code?

No, I'm not sure what I should be looking for. I thought perhaps this
was a known limitation; should I report this as a bug?

bug#28658: 27.0.50; [PATCH] double/triple clicking in xterm-mouse-mode doesn't respect mouse position

[Eli Zaretskii]

> From: Alex <agrambot@gmail.com>
> Cc: 28658@debbugs.gnu.org
> Date: Sun, 08 Oct 2017 20:37:20 -0600
> 
> No, I'm not sure what I should be looking for. I thought perhaps this
> was a known limitation; should I report this as a bug?

Maybe first mention this on emacs-devel, perhaps someone already knows
about this.

bug#28658: 27.0.50; [PATCH] double/triple clicking in xterm-mouse-mode doesn't respect mouse position

[Eli Zaretskii]

> From: Alex <agrambot@gmail.com>
> Date: Sat, 30 Sep 2017 16:06:34 -0600
> 
> 1. emacs -Q -nw
> 2. M-x xterm-mouse-mode RET
> 3. Click once, then quickly move the cursor to a different word and click again.
> 4. Notice that the 2nd click was registered as a double click even
> though the cursor moved.
> 5. Click once, and quickly perform step 3 again. Notice that the 3rd
> click was registered as a triple click even though the cursor moved.
> 
> I've attached a patch that fixes this behaviour.

Thanks, I have a comment to this, but in general this is the right
fix, IMO.

> It would be nice if it worked with pixel positions rather than
> character positions, but I'm not sure how to do that in a terminal
> Emacs.

You can't: TTY frames cannot discern screen positions with resolution
of more than 1 character.

> @@ -290,12 +292,14 @@ xterm-mouse-event
>                (xterm-mouse--set-click-count event click-count)))
>             ((not last-time) nil)
>             ((and (> double-click-time (* 1000 (- this-time last-time)))
> +                 (eq x last-x)
> +                 (eq y last-y)

IMO, 'eq' is not right here: this test should obey the value of
double-click-fuzz, whose units on TTY frames are 1/8 of a character.

bug#28658: 27.0.50; [PATCH] double/triple clicking in xterm-mouse-mode doesn't respect mouse position

[Alex]

Eli Zaretskii <eliz@gnu.org> writes:


>> It would be nice if it worked with pixel positions rather than
>> character positions, but I'm not sure how to do that in a terminal
>> Emacs.
>
> You can't: TTY frames cannot discern screen positions with resolution
> of more than 1 character.
>
>> @@ -290,12 +292,14 @@ xterm-mouse-event
>>                (xterm-mouse--set-click-count event click-count)))
>>             ((not last-time) nil)
>>             ((and (> double-click-time (* 1000 (- this-time last-time)))
>> +                 (eq x last-x)
>> +                 (eq y last-y)
>
> IMO, 'eq' is not right here: this test should obey the value of
> double-click-fuzz, whose units on TTY frames are 1/8 of a character.

I don't understand how to use double-click-fuzz in TTY frames. You said
that TTY frames can't discern screen position differences of less than a
character, so then why are the units 1/8th of a character?

Is there a way to get the mouse coordinate position in 1/8ths of a
character to use double-click-fuzz?

bug#28658: 27.0.50; [PATCH] double/triple clicking in xterm-mouse-mode doesn't respect mouse position

[Eli Zaretskii]

> From: Alex <agrambot@gmail.com>
> Cc: 28658@debbugs.gnu.org
> Date: Thu, 05 Oct 2017 18:14:39 -0600
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> 
> >> @@ -290,12 +292,14 @@ xterm-mouse-event
> >>                (xterm-mouse--set-click-count event click-count)))
> >>             ((not last-time) nil)
> >>             ((and (> double-click-time (* 1000 (- this-time last-time)))
> >> +                 (eq x last-x)
> >> +                 (eq y last-y)
> >
> > IMO, 'eq' is not right here: this test should obey the value of
> > double-click-fuzz, whose units on TTY frames are 1/8 of a character.
> 
> I don't understand how to use double-click-fuzz in TTY frames. You said
> that TTY frames can't discern screen position differences of less than a
> character, so then why are the units 1/8th of a character?

I don't know why the units are 1/8th of a character, perhaps so that a
user could set the same value for both GUI and TTY frames.  In any
case, dividing the value of double-click-fuzz by 8 before comparing
with coordinate differences is easy enough, no?

> Is there a way to get the mouse coordinate position in 1/8ths of a
> character to use double-click-fuzz?

I don't think you can get sub-character resolution, but that doesn't
mean double-click-fuzz cannot be obeyed: a user could set the value to
16, in which case a 2-character distance should still be okay.  With
the default value of 3, the second click should indeed be on the same
character, so it will be equivalent to your eq test, but that's only
the default.

bug#28658: 27.0.50; [PATCH] double/triple clicking in xterm-mouse-mode doesn't respect mouse position

[Alex]

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

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Alex <agrambot@gmail.com>
>> Cc: 28658@debbugs.gnu.org
>> Date: Thu, 05 Oct 2017 18:14:39 -0600
>> 
>> Eli Zaretskii <eliz@gnu.org> writes:
>> 
>> 
>> >> @@ -290,12 +292,14 @@ xterm-mouse-event
>> >>                (xterm-mouse--set-click-count event click-count)))
>> >>             ((not last-time) nil)
>> >>             ((and (> double-click-time (* 1000 (- this-time last-time)))
>> >> +                 (eq x last-x)
>> >> +                 (eq y last-y)
>> >
>> > IMO, 'eq' is not right here: this test should obey the value of
>> > double-click-fuzz, whose units on TTY frames are 1/8 of a character.
>> 
>> I don't understand how to use double-click-fuzz in TTY frames. You said
>> that TTY frames can't discern screen position differences of less than a
>> character, so then why are the units 1/8th of a character?
>
> I don't know why the units are 1/8th of a character, perhaps so that a
> user could set the same value for both GUI and TTY frames.  In any
> case, dividing the value of double-click-fuzz by 8 before comparing
> with coordinate differences is easy enough, no?

Yes, I was just confused about the units, but that makes sense. Though
in my GTK Emacs, (frame-char-width) returns 9 instead of 8 for me...

Anyway, here's the patch:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: xterm --]
[-- Type: text/x-diff, Size: 1866 bytes --]

From e9c7aca2ade951e03f67db66bb8d2608d740936d Mon Sep 17 00:00:00 2001
From: Alexander Gramiak <agrambot@gmail.com>
Date: Sat, 30 Sep 2017 15:47:56 -0600
Subject: [PATCH] Increase xterm click count only within double-click-fuzz

* lisp/xt-mouse.el (xterm-mouse-event): Save the last click's position
and check it against the current click's position.
---
 lisp/xt-mouse.el | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/lisp/xt-mouse.el b/lisp/xt-mouse.el
index d268e1a3fe..d704cfa4e8 100644
--- a/lisp/xt-mouse.el
+++ b/lisp/xt-mouse.el
@@ -278,6 +278,8 @@ xterm-mouse-event
                (last-name (symbol-name last-type))
                (last-time (nth 1 last-click))
                (click-count (nth 2 last-click))
+               (last-x (nth 3 last-click))
+               (last-y (nth 4 last-click))
                (this-time (float-time))
                (name (symbol-name type)))
           (cond
@@ -292,12 +294,16 @@ xterm-mouse-event
                  double-click-time
                  (or (eq double-click-time t)
                      (> double-click-time (* 1000 (- this-time last-time))))
+                 (<= (abs (- x last-x))
+                     (/ double-click-fuzz 8))
+                 (<= (abs (- y last-y))
+                     (/ double-click-fuzz 8))
                  (equal last-name (replace-match "" t t name)))
             (setq click-count (1+ click-count))
             (xterm-mouse--set-click-count event click-count))
            (t (setq click-count 1)))
           (set-terminal-parameter nil 'xterm-mouse-last-click
-                                  (list type this-time click-count)))
+                                  (list type this-time click-count x y)))
 
         (set-terminal-parameter nil 'xterm-mouse-x x)
         (set-terminal-parameter nil 'xterm-mouse-y y)
-- 
2.14.2

bug#28658: 27.0.50; [PATCH] double/triple clicking in xterm-mouse-mode doesn't respect mouse position

[Eli Zaretskii]

> From: Alex <agrambot@gmail.com>
> Cc: 28658@debbugs.gnu.org
> Date: Sat, 07 Oct 2017 15:57:04 -0600
> 
> > I don't know why the units are 1/8th of a character, perhaps so that a
> > user could set the same value for both GUI and TTY frames.  In any
> > case, dividing the value of double-click-fuzz by 8 before comparing
> > with coordinate differences is easy enough, no?
> 
> Yes, I was just confused about the units, but that makes sense. Though
> in my GTK Emacs, (frame-char-width) returns 9 instead of 8 for me...

Yes, the 8 thing cannot be anything but an approximation.

> Anyway, here's the patch:

LGTM, thanks.  Please mention the bug number in the log message when
you push (to the emacs-26 branch).

bug#28658: 27.0.50; [PATCH] double/triple clicking in xterm-mouse-mode doesn't respect mouse position

[Alex]

Eli Zaretskii <eliz@gnu.org> writes:

> LGTM, thanks.  Please mention the bug number in the log message when
> you push (to the emacs-26 branch).

Pushed.