Better Mouse Support #1387

RyannDaGreat · 2021-03-06T00:05:48Z

In this pull request, I added more mouse support functionality, including the ability to discriminate which mouse button was pressed as well as reporting click-drags, and mouse movements when no mouse button is pressed. Also improved mouse support while text editing: when you click and drag, your selection will be shown in realtime (as opposed to having to release the mouse press to see the resulting selection). To see this functionality in action, try running examples/full-screen/text-editor.py, write some text, then click and drag over that text to select it.

To help you understand the difference more clearly, I've made a youtube video showing what my pull request does:

https://www.youtube.com/watch?v=Qe76evU5VgA

…riminate which mouse button was pressed as well as reporting click-drags, and mouse movements when no mouse button is pressed. Also improved mouse support while text editing: when you click and drag, your selection will be shown in realtime (as opposed to having to release the mouse press to see the resulting selection). To see this functionality in action, try running examples/full-screen/text-editor.py, write some text, then click and drag over that text to select it.

mcclurem · 2021-03-10T22:28:31Z

I literally /just/ hacked together a crappier version of this PR. I would really love to see this merged

jonathanslenders · 2021-03-11T11:51:12Z

Thanks a lot! This sounds great. I hope to find some time soon to review and merge this.

jonathanslenders

Looks good already! But some improvements would be good.

jonathanslenders · 2021-03-22T18:54:58Z

prompt_toolkit/key_binding/bindings/mouse.py

@@ -10,6 +15,30 @@
    "load_mouse_bindings",
 ]

+# fmt: off


Could you remove fmt: off markings. I prefer to not use them, sorry.

Actually, discard this question. It's fine.

jonathanslenders · 2021-03-22T18:56:22Z

prompt_toolkit/key_binding/bindings/mouse.py

+
+            # TODO: Is it possible to add modifiers here?
+            # fmt: off
+            mouse_button,mouse_event_type,mouse_modifier = {


Can this lookup table become a global constant? (The function is getting rather long otherwise).

jonathanslenders · 2021-03-22T18:56:47Z

prompt_toolkit/key_binding/bindings/mouse.py

-                }.get((mouse_event, m))
+                # fmt: off
+                # flake8: noqa E201
+                mouse_button,mouse_event_type,mouse_modifier = {


Can this become a global constant as well?

Maybe fmt: off is justified here to align the comments.

jonathanslenders · 2021-03-22T18:58:37Z

prompt_toolkit/mouse_events.py

+    SHIFT_CONTROL     = (MouseModifierKey.SHIFT,                     MouseModifierKey.CONTROL,)
+    ALT_CONTROL       = (                       MouseModifierKey.ALT,MouseModifierKey.CONTROL,)
+    SHIFT_ALT_CONTROL = (MouseModifierKey.SHIFT,MouseModifierKey.ALT,MouseModifierKey.CONTROL,)
+    UNKNOWN_MODIFIER  = ("UNKNOWN") # This is used if we're not sure what modifiers are being used, if any


This is not a tuple.

One could alternatively use IntFlag for Modifiers:

class MouseModifierKey(IntFlag): SHIFT = 1 ALT = 2 CONTROL = 4 class MouseModifier(Enum): NO_MODIFIER = 0 SHIFT = MouseModifierKey.SHIFT ALT = MouseModifierKey.ALT SHIFT_ALT = MouseModifierKey.SHIFT | MouseModifierKey.ALT CONTROL = MouseModifierKey.CONTROL SHIFT_CONTROL = MouseModifierKey.SHIFT | MouseModifierKey.CONTROL ALT_CONTROL = MouseModifierKey.ALT | MouseModifierKey.CONTROL SHIFT_ALT_CONTROL = MouseModifierKey.SHIFT | MouseModifierKey.ALT | MouseModifierKey.CONTROL UNKNOWN_MODIFIER = "UNKNOWN"

If that's cleaner or not, I don't know.

jonathanslenders · 2021-03-22T19:00:21Z

prompt_toolkit/mouse_events.py

+        position: Point,
+        event_type: MouseEventType,
+        button: MouseButton,
+        modifier: MouseModifier,


I don't think it makes sense for modifier to be an enum. Set[MouseModifier] seems more logical.

Set[MouseModifier] makes more semantic sense, but the reason I decided to do it this way is because Set[MouseModifier] isn't hashable - meaning you can't use Set[MouseModifier] as keys in a dictionary to branch off efficiently

We have frozenset for that. ;)

While I merge it, I'll probably do some polishing, and use frozen sets. If that's fine for you.

I just looked into that...yeah I think that would work well! I don't know how I missed that feature lol:) I don't think changing them from tuples to frozensets will affect the functionality of the code, so I think it should be safe to change it if you want

jonathanslenders · 2021-03-22T19:00:57Z

prompt_toolkit/mouse_events.py

+    LEFT           = "LEFT"
+    MIDDLE         = "MIDDLE"
+    RIGHT          = "RIGHT"
+    NO_BUTTON      = ""                # When we're scrolling, or just moving the mouse and not pressing a button, mouse_event.button=="". The reason it's an empty string is so that bool(MouseButton.NO_BUTTON)==False


Can you put the comments above the constants, and wrap them at 80 characters?

I don't like that people would rely on bool(MouseButton.NO_BUTTON)==False. I don't think mypy would approve it either. Can you make it NO_BUTTON = "NO_BUTTON"?

jonathanslenders · 2021-03-22T19:01:40Z

prompt_toolkit/key_binding/bindings/mouse.py

+                    modifier=mouse_modifier,
+                )
+            )
+            # print(MouseEvent(position=Point(x=x, y=y), event_type=mouse_event_type, button=mouse_button, modifier=mouse_modifier)) # Uncomment to debug mouse events


Can you remove this line?

No button mouse drag events have a lowercase m for me. (MOUSE_MOVE instead of MOUSE_DRAG might be a better name?)

RyannDaGreat · 2021-06-20T07:43:28Z

Hi Jonathan, I made a revision to this pull request in the 5th commit - please take a look!

RyannDaGreat · 2021-06-20T08:02:33Z

I also added a small new feature, which gives dropdown menus more interactivity! It acts more like windows or mac now; when you click the 'file' menu for example, the menu items will be highlighted as you drag your mouse down (even if you don't click). Likewise, you can now press your mouse down on a 'file' menu, and release it on the 'save' menu item - and it will trigger (no need to click twice, like in Ubuntu, Windows or Mac). This small addition is a good use case for this better mouse support.

I've made a video to demonstrate this functionality:
https://www.youtube.com/watch?v=zql3G3B9LI8

salt-die

If this gets merged, one can add support to windows with the extra modifiers by updating ConsoleInputReader._handle_mouse in win32.py (partial implementation as an example):

    def _handle_mouse(self, ev: MOUSE_EVENT_RECORD) -> List[KeyPress]:
        FROM_LEFT_1ST_BUTTON_PRESSED = 0x0001
        RIGHTMOST_BUTTON_PRESSED =  0x0002
        FROM_LEFT_2ND_BUTTON_PRESSED = 0x0004

        RIGHT_ALT_PRESSED = 0x0001
        LEFT_ALT_PRESSED = 0x0002
        RIGHT_CTRL_PRESSED = 0x0004
        LEFT_CTRL_PRESSED = 0x0008
        SHIFT_PRESSED = 0x0010

        MOUSE_MOVED = 0x0001
        MOUSE_WHEELED = 0x0004

        # Event type
        if ev.EventFlags & MOUSE_MOVED:
            event_type = MouseEventType.MOUSE_MOVE
        elif ev.EventFlags & MOUSE_WHEELED:
            if ev.ButtonState > 0:
                event_type = MouseEventType.SCROLL_UP
            else:
                event_type = MouseEventType.SCROLL_DOWN
        elif not ev.ButtonState:
            event_type = MouseEventType.MOUSE_UP
        else:
            event_type = MouseEventType.MOUSE_DOWN

        # Buttons
        if not ev.ButtonState:
            button = MouseButton.NO_BUTTON
        elif ev.ButtonState & FROM_LEFT_1ST_BUTTON_PRESSED:
            button = MouseButton.LEFT
        elif ev.ButtonState & RIGHTMOST_BUTTON_PRESSED:
            button = MouseButton.RIGHT
        # More buttons here:
        #     https://docs.microsoft.com/en-us/windows/console/mouse-event-record-str?redirectedfrom=MSDN
        # For now, just assume middle-mouse button.
        else:
            button = MouseButton.MIDDLE

        # Modifiers (If using IntFlag Enum for modifier keys)
        mods = 0
        if ev.ControlKeyState & LEFT_ALT_PRESSED or ev.ControlKeyState & RIGHT_ALT_PRESSED:
            mods |= MouseModifierKey.ALT
        if ev.ControlKeyState & LEFT_CTRL_PRESSED or ev.ControlKeyState & RIGHT_CTRL_PRESSED:
            mods |= MouseModifierKey.CONTROL
        if ev.ControlKeyState & SHIFT_PRESSED:
            mods |= MouseModifierKey.SHIFT

        modifier = MouseModifier(mods)
        ...
        # Package data somehow

salt-die · 2021-08-01T08:55:58Z

prompt_toolkit/key_binding/bindings/mouse.py

+                    modifier=mouse_modifier,
+                )
+            )
+            # print(MouseEvent(position=Point(x=x, y=y), event_type=mouse_event_type, button=mouse_button, modifier=mouse_modifier)) # Uncomment to debug mouse events


No button mouse drag events have a lowercase m for me. (MOUSE_MOVE instead of MOUSE_DRAG might be a better name?)

RyannDaGreat · 2021-08-02T17:38:07Z

@salt-die In the newest commit, I actually changed MOUSE_DRAG to MOUSE_MOVE lol - I agree, MOUSE_MOVE is probably a better name. As for using numbers to represent the mouse modifiers, you make a good point - it would be nice to combine them using | . However, that would also make the mouse modifiers unreadable if you tried to print them without internal knowledge of how prompt_toolkit works. For example, if somebody prints ALT_CONTROL for debugging and gets 6, it doesn't tell you very much. I think we should leave that decision to @jonathanslenders

salt-die · 2021-08-02T20:42:25Z

Printing isn't that bad with IntFlag enums, one gets a slightly better repr than just an int:

In [61]: MouseModifier.SHIFT_ALT
Out[61]: <MouseModifier.SHIFT_ALT: <MouseModifierKey.ALT|SHIFT: 3>>

In [62]: MouseModifier.SHIFT_ALT_CONTROL
Out[62]: <MouseModifier.SHIFT_ALT_CONTROL: <MouseModifierKey.CONTROL|ALT|SHIFT: 7>>

jonathanslenders · 2021-08-05T07:46:49Z

Hi @RyannDaGreat , this looks great! Is there any chance you can rebase on the master branch? There are a lot of formatting changes, which makes the code hard to review. I'm not sure what's the easiest way to rebase.

RyannDaGreat · 2021-08-05T08:04:26Z

Hi @jonathanslenders! I've never rebased anything before, so this is a bit new to me...would this be done as an additional commit? (I just started looking up documentation at https://git-scm.com/book/en/v2/Git-Branching-Rebasing)

jonathanslenders · 2021-08-05T08:26:17Z

@RyannDaGreat,

In that case, maybe leave it like it is. I'll see if I get some time, and can do it myself in the coming week.

RyannDaGreat · 2021-08-05T08:32:29Z

Ok, thank you! Let me know if there's anything else I can do to help.
On a side note (slightly off topic from this commit but still fairly relevant), I was wondering if you'd also be open to other features being integrated with the new mouse controls? For example, buttons that are highlighted when you mouse over them, or right clicking text to get a context menu etc. In particular, a context menu like you get when right clicking in tmux

jonathanslenders · 2021-08-05T08:47:15Z

"I was wondering if you'd also be open to other features being integrated with the new mouse controls?"

Yes, I am. But a few important things to keep into account before I can merge these things:

Performance is important. No new functionality can impact the performance of current users. Same for startup time.
The default experience should probably be "boring". Special features like this should be enabled if the user wants them.
We should not be opinionated about the experience of the user of this library. I mean, hover effects for buttons are fine (because that's only enabling/disabling a certain style which is easy to customize). But context menus are hardly not opinionated. (which items should or should not be there). The hover effect over the menus that you showed sounds nice too.
It's always better if certain functionality can be built on top of prompt_toolkit, rather then in prompt_toolkit. Better to expose APIs so that people can build their own widgets with certain context menus, etc...
I'm much in favor of experimenting, but prefer to move slowly when it comes to integrating new functionality in the core library now. (Part of it is time constraint on my side, but also maintenance burden that comes with features that are poorly implemented. So, I prefer things to be "right" before merging them. This is also the reason that I prefer an extensible architecture where customization can be built on top, external to the library.)

Hope that makes sense.

jonathanslenders · 2021-08-19T14:18:43Z

I rebased these changes on master. See this PR:
#1481

I'll try to take it from here.

SqrtRyan added 2 commits March 5, 2021 18:41

Made the code flake8-compliant

924447c

RyannDaGreat force-pushed the master branch from 4c44fdf to 924447c Compare March 6, 2021 00:44

SqrtRyan added 2 commits March 5, 2021 20:00

Fixed mypy issues

abb94dd

Formatted with black

9a0db33

jonathanslenders requested changes Mar 22, 2021

View reviewed changes

Added requested changes

874bf2d

SqrtRyan added 2 commits June 20, 2021 03:58

Gave dropdown menus more interactivity with the new mouse support

0f3cf16

Formatted with black

9fe62be

RyannDaGreat requested a review from jonathanslenders July 22, 2021 18:42

salt-die reviewed Aug 1, 2021

View reviewed changes

jonathanslenders mentioned this pull request Aug 19, 2021

Improved mouse support #1481

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Better Mouse Support #1387

Better Mouse Support #1387

RyannDaGreat commented Mar 6, 2021

mcclurem commented Mar 10, 2021

jonathanslenders commented Mar 11, 2021

jonathanslenders left a comment

jonathanslenders Mar 22, 2021

jonathanslenders Aug 5, 2021

jonathanslenders Mar 22, 2021

RyannDaGreat Jun 20, 2021

jonathanslenders Mar 22, 2021

RyannDaGreat Jun 20, 2021

jonathanslenders Mar 22, 2021

RyannDaGreat Jun 20, 2021

salt-die Aug 1, 2021 •

edited

Loading

jonathanslenders Mar 22, 2021

RyannDaGreat Apr 7, 2021

jonathanslenders Aug 5, 2021

RyannDaGreat Aug 5, 2021 •

edited

Loading

jonathanslenders Mar 22, 2021

jonathanslenders Mar 22, 2021

RyannDaGreat Jun 20, 2021

jonathanslenders Mar 22, 2021

RyannDaGreat Jun 20, 2021

salt-die Aug 1, 2021

RyannDaGreat commented Jun 20, 2021

RyannDaGreat commented Jun 20, 2021 •

edited

Loading

salt-die left a comment •

edited

Loading

salt-die Aug 1, 2021

RyannDaGreat commented Aug 2, 2021

salt-die commented Aug 2, 2021

jonathanslenders commented Aug 5, 2021

RyannDaGreat commented Aug 5, 2021

jonathanslenders commented Aug 5, 2021

RyannDaGreat commented Aug 5, 2021 •

edited

Loading

jonathanslenders commented Aug 5, 2021

jonathanslenders commented Aug 19, 2021 •

edited

Loading

Better Mouse Support #1387

Are you sure you want to change the base?

Better Mouse Support #1387

Conversation

RyannDaGreat commented Mar 6, 2021

mcclurem commented Mar 10, 2021

jonathanslenders commented Mar 11, 2021

jonathanslenders left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

salt-die Aug 1, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

RyannDaGreat Aug 5, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

RyannDaGreat commented Jun 20, 2021

RyannDaGreat commented Jun 20, 2021 • edited Loading

salt-die left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

RyannDaGreat commented Aug 2, 2021

salt-die commented Aug 2, 2021

jonathanslenders commented Aug 5, 2021

RyannDaGreat commented Aug 5, 2021

jonathanslenders commented Aug 5, 2021

RyannDaGreat commented Aug 5, 2021 • edited Loading

jonathanslenders commented Aug 5, 2021

jonathanslenders commented Aug 19, 2021 • edited Loading

salt-die Aug 1, 2021 •

edited

Loading

RyannDaGreat Aug 5, 2021 •

edited

Loading

RyannDaGreat commented Jun 20, 2021 •

edited

Loading

salt-die left a comment •

edited

Loading

RyannDaGreat commented Aug 5, 2021 •

edited

Loading

jonathanslenders commented Aug 19, 2021 •

edited

Loading