From: Jared Finder via "Emacs development discussions." <emacs-devel@gnu.org>
To: Stefan Monnier <monnier@iro.umontreal.ca>
Cc: "Jared Finder via \"Emacs development discussions.\""
<emacs-devel@gnu.org>
Subject: Re: mouse-face and help echo support for xterm mouse
Date: Thu, 05 Nov 2020 11:58:09 -0800 [thread overview]
Message-ID: <946d9ea094642758037d1881a97e8d0c@finder.org> (raw)
In-Reply-To: <jwvzh3vlw42.fsf-monnier+emacs@gnu.org>
On 2020-11-05 6:45 am, Stefan Monnier wrote:
>>
>> diff --git a/lisp/xt-mouse.el b/lisp/xt-mouse.el
>> index f9c08f9a17..37550276f8 100644
>> --- a/lisp/xt-mouse.el
>> +++ b/lisp/xt-mouse.el
>> @@ -77,6 +77,7 @@ xterm-mouse-translate-1
>> (copy-sequence event))
>> vec)
>> (is-move
>> + (xterm-mouse--handle-mouse-motion)
>> (if track-mouse vec
>> ;; Mouse movement events are currently supposed to be
>> ;; suppressed. Return no event.
>> @@ -106,8 +107,16 @@ xterm-mouse-translate-1
>> (if (null track-mouse)
>> (vector drag)
>> (push drag unread-command-events)
>> + (xterm-mouse--handle-mouse-motion)
>> (vector (list 'mouse-movement ev-data))))))))))))
>
> Hmm... `ev-data` includes the X/Y position, right?
> Could we arrange to pass *that* data directly to
> `xterm-mouse--handle-mouse-motion` rather than go through
> (terminal-parameter frame 'xterm-mouse-x/y)? It likely won't make
> much difference in practice, but it would make the data flow more
> clear,
> I think.
Using ev-data's x,y would add significant complexity. ev-data is a
posn, which is window part relative, not frame relative.
>> +(defun xterm-mouse--handle-mouse-motion ()
>> + "Handle mouse motion that was just generated for XTerm mouse."
>> + (let ((frame (selected-frame)))
>> + (handle-lisp-mouse-motion frame
>> + (terminal-parameter frame
>> 'xterm-mouse-x)
>> + (terminal-parameter frame
>> 'xterm-mouse-y))))
>
> This is the only caller to `handle-lisp-mouse-motion` and that function
> has a "default frame to selected-frame" feature, so you could pass nil
> instead of `frame`. Better yet, drop that frame argument altogether.
> And I think the function's name should make it clear it's for internal
> use only, or otherwise try to use some prefix that indicates it's
> related to the display. Like `display-update-for-mouse-motion`?
Is there a way I can name it that makes it clear this is an internal
function and we may change the arguments in the future? I was hoping
that this function would work with background frames across different
TTYs in case I can figure out how to get that working.
> [ I'm reminded here of the tension between using "mouse-motion" because
> it's
> shorter and using "mouse-movement" because that's the name of the
> event. ]
I squared this circle by using "mouse motion" for the concept and "mouse
movement" for the event. I could rename to mouse-movement if you'd
prefer.
>> +This function should be called when Lisp code detects the mouse has
>> +moved, even if `track-mouse' is nil. This handles updates that do
>> not
>> +not rely on input events such as updating display for mouse-face
>
> "not not"
Done.
>> +proprties or updating the help echo text. */)
> ^^
> e
Done.
>> + (Lisp_Object frame, Lisp_Object mouse_x, Lisp_Object mouse_y)
>> +{
>> + if (NILP (frame))
>> + frame = selected_frame;
>> +
>> + update_mouse_position (XFRAME (frame), XFIXNUM (mouse_x), XFIXNUM
>> (mouse_y));
>> + return Qnil;
>> +}
>
> This will crash and burn if `frame` is an integer (and the XFIXNUM
> won't
> crash and burn but they should also be preceded by CHECK_FIXNUMs).
Done.
>> if (event->type & (GPM_MOVE | GPM_DRAG))
>> {
> [...]
>> + /* Has the mouse moved off the glyph it was on at the last
>> sighting? */
>> + if (event->x != last_mouse_x || event->y != last_mouse_y)
>> + {
>> + last_mouse_x = event->x;
>> + last_mouse_y = event->y;
>> + f->mouse_moved = 1;
>
> Would it make sense to try and add this test to the "generic" part of
> the code?
This is not possible without much further work. These are all tied with
the C mouse event path: last_mouse_x, last_mouse_y, and
frame.mouse_moved are all used by the mouse position hook and integrated
with keyboard.c's mouse event generation.
Making this shared would require changing the way xterm-mouse currently
reports all mouse events to not return data through input-decode-map and
instead creating a Lisp interface to the C mouse event generation logic.
I tried playing around with this locally and it did seem like there is
a path to make it work, but it would be a much bigger effort.
As there's no logic changes, I'll wait for my questions above to get
addressed before sending an updated patch.
-- MJF
next prev parent reply other threads:[~2020-11-05 19:58 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-11-01 5:46 mouse-face and help echo support for xterm mouse Jared Finder via Emacs development discussions.
2020-11-01 13:39 ` Stefan Monnier
2020-11-01 15:56 ` Jared Finder via Emacs development discussions.
2020-11-04 6:54 ` Jared Finder via Emacs development discussions.
2020-11-04 14:13 ` Stefan Monnier
2020-11-04 15:46 ` Eli Zaretskii
2020-11-04 15:56 ` Stefan Monnier
2020-11-04 17:54 ` Jared Finder via Emacs development discussions.
2020-11-04 18:47 ` Stefan Monnier
2020-11-04 18:51 ` Eli Zaretskii
2020-11-04 19:05 ` Stefan Monnier
2020-11-04 19:10 ` Jared Finder via Emacs development discussions.
2020-11-04 15:19 ` Eli Zaretskii
2020-11-05 8:15 ` Jared Finder via Emacs development discussions.
2020-11-05 14:45 ` Stefan Monnier
2020-11-05 19:58 ` Jared Finder via Emacs development discussions. [this message]
2020-11-05 20:18 ` Stefan Monnier
2020-11-06 5:23 ` Jared Finder via Emacs development discussions.
2020-11-06 6:00 ` Eli Zaretskii
2020-11-06 6:46 ` Jared Finder via Emacs development discussions.
2020-11-06 7:39 ` Eli Zaretskii
2020-11-07 1:22 ` Jared Finder via Emacs development discussions.
2020-11-14 12:38 ` Eli Zaretskii
2020-11-14 12:35 ` Eli Zaretskii
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: https://www.gnu.org/software/emacs/
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=946d9ea094642758037d1881a97e8d0c@finder.org \
--to=emacs-devel@gnu.org \
--cc=jared@finder.org \
--cc=monnier@iro.umontreal.ca \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/emacs.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).