Mouse AnyEvent tracking (1003) -- work in progress by AutumnMeowMeow · Pull Request #3538 · zellij-org/zellij

AutumnMeowMeow · 2024-08-03T15:41:25Z

PR for more detailed discussion, for #1679

Current bit compiles, passes tests, fails clippy (not my fault, other parts of zellij-utils doing that).

Things to do:

Fix broken mouse Release events
Decide how Motion events will be exposed to zellij code, i.e. which Action:: items will it have?
Implement Motion events
Fix broken mouse text selection / copy-paste
Add CSI ? 1003 support inside the terminal pane
Test Normal tracking in vttest
Test ButtonEvent tracking in vttest
Test AnyEvent tracking in vttest
Fixup other code style issues (I still suck at Rust)

…se button state

AutumnMeowMeow · 2024-08-03T15:52:16Z

@imsnif

You can see my TODOs on top. I will try to minimize commits to this PR so it doesn't constantly spin up the tests.

Initial questions:

Does it make sense what I have here so far? Moving away from a "single button down" --> "held" --> "release" model, instead to "button(s) down" --> "button(s) maybe released or dragged, no buttons, etc." I would like to carry this simpler model down to the terminal/grid layer, replacing the six {left/right/middle}_button_{signal/release_signal} functions with three functions {press/release/motion}. On the plugins layer all the existing Actions could stay as-is so as not to break existing code.
What Action's would you like to see for Motion events? (My vote would be a Action::MouseMotion(point, buttons).)

imsnif · 2024-08-05T15:59:45Z

Hey @AutumnMeowMeow !

The direction looks very good. We do not enforce clippy anywhere, so all good.

* Does it make sense what I have here so far?  Moving away from a "single button down" --> "held" --> "release" model, instead to "button(s) down" --> "button(s) maybe released or dragged, no buttons, etc."  I would like to carry this simpler model down to the terminal/grid layer, replacing the six {left/right/middle}_button_{signal/release_signal} functions with three functions {press/release/motion}.  On the plugins layer all the existing Actions could stay as-is so as not to break existing code.

Sounds perfect. From my side, as long as the plugin API (and especially the protobuf layer) doesn't change we're not breaking anything, so should be good. It sounds like this structure is much more in-tune with the way these events are encoded as seen in your changes, so let's go with it.

* What Action's would you like to see for Motion events?  (My vote would be a Action::MouseMotion(point, buttons).)

Sounds good to me! Note that when you get to serializing these actions to protobuffs, you can just ignore them as unsupported actions here: https://github.com/zellij-org/zellij/blob/main/zellij-utils/src/plugin_api/action.rs#L1295

I'm excited about this feature! Already thinking of how to use it in the UI.

imsnif · 2024-08-05T15:34:58Z

+        // unfortunate side effect of the pre-SGR-encoded X10 mouse
+        // protocol design in which release events don't carry
+        // information about WHICH button(s) were released, so we have
+        // to maintain a wee bit of state in between events.


Honestly, even this is much better than what we've been doing up until now with the HeldMouse stuff.

side terminal(s) in, including protobuf. Removed "held" mouse actions. Currently commented out calls to left/middle/right-click/release -- need to fix this though, as selection/copy-paste are broken too. cargo build/test/run works OK. cargo xtask build/test/run fails, unable to find crate input::mouse.

AutumnMeowMeow · 2024-08-06T20:28:13Z

@imsnif OK most of the core new function is here. :) Some things are broken though, and I could use your pointers. (Also converted this to draft in the futile hope it would not spin up the whole CI stuff on every commit...sigh.)

cargo build/run/test work fine. However cargo xtask build/run/test do not. They cannot see the input::mouse crate, no idea why. Any ideas on how to fix?
I temporarily commented out the other left/right/middle-click functions, which now breaks text selection of course. But along the way I also removed the MouseHold stuff, and see a few decently large functions that got axed in zellij-server-src/tab/mod.rs. Do we actually need those? Or would those functions still work via only LeftClick / LeftRelease / etc.?
Somewhere along the way the mouse position is getting a couple off-by-ones, i.e. not 0-based. I suspect that is an artifact of the find_active_tab stuff I cargo culted from the left_click code, e.g. failing to properly handle the conversion from absolute coordinates returned by termwiz to the relative-pane coordinates needed before sending to the terminal.

Thank you for your help! :-)

AutumnMeowMeow · 2024-08-06T20:40:51Z

+                self.mouse_buttons_value_sgr(event),
+                // AZL: Why is event.position not staying 0-based on
+                // both axes?
+                event.position.column(),


I see that I missed the relative coordinate conversion in handle_mouse_event(). I'll fix that.

AutumnMeowMeow · 2024-08-06T20:43:57Z

-        }
-        Ok(false) // we shouldn't even get here, but might as well not needlessly render if we do
-    }
-


The three functions above are decently large. Is there logic anywhere else to handle changing the selection buffer size as the mouse is dragged around? If not, then we will need to put that in handle_mouse_event().

AutumnMeowMeow · 2024-08-06T20:46:01Z

+    pub shift: bool,
+    pub alt: bool,
+    pub ctrl: bool,
+


Also FYI that Swing and WIN32 both expose keyboard modifiers with mouse events. Soooo when zellij is running natively on Windows we'll have the infrastructure to do things like Ctrl-Click / Alt-Drag / etc. :-)

Honestly, even Alt + mouse_click would be amazing. I'm thinking of using it for multiple selection of panes.

imsnif · 2024-08-08T12:32:19Z

Hey @AutumnMeowMeow - sorry for the delay in getting to this - I'd like to give this a priority but I had a bit of a hectic day yesterday.

cargo build/run/test work fine. However cargo xtask build/run/test do not. They cannot see the input::mouse crate, no idea why. Any ideas on how to fix?

Brief explanation: this is happening because the Action struct is shared across the webassembly boundary to plugins. Termwiz has some dependency that doesn't play well with wasm (right now I don't fully remember which one) and since the mouse module depends on termwiz, we stub it out here: https://github.com/zellij-org/zellij/blob/main/zellij-utils/src/input/mod.rs#L12

Suggested solution: I think the cleanest way to get around this is to remove the mouse module's dependency on termwiz. I think we can do that if we make the termwiz_mouse_convert and from_termwiz functions independent of the mouse module. We could for example (just from a brief look, maybe you have an idea that makes more sense!) change their signatures to:

fn termwiz_mouse_convert(original_event: &mut MouseEvent, event: &termwiz::input::MouseEvent)
fn from_termwiz(old_event: &mut MouseEvent, event: termwiz::input::MouseEvent) -> MouseEvent

And then place them in the input_handler.rs file where they are used as bare functions. What do you think?

I temporarily commented out the other left/right/middle-click functions, which now breaks text selection of course. But along the way I also removed the MouseHold stuff, and see a few decently large functions that got axed in zellij-server-src/tab/mod.rs. Do we actually need those? Or would those functions still work via only LeftClick / LeftRelease / etc.?

I think we should be able to use these functions as is (maybe even removing the whole is_repeated and last_position logic) with the new motions. Here we control the movement of floating panes and text selection (basically, they detect whether we're holding the mouse on a the frame of a floating pane or inside a terminal pane). Does that answer your question?

Somewhere along the way the mouse position is getting a couple off-by-ones, i.e. not 0-based. I suspect that is an artifact of the find_active_tab stuff I cargo culted from the left_click code, e.g. failing to properly handle the conversion from absolute coordinates returned by termwiz to the relative-pane coordinates needed before sending to the terminal.

I think you found this in your other comment, right?

AutumnMeowMeow · 2024-08-29T20:59:17Z

Refactoring termwiz_mouse_convert and from_termwiz got the builds ~~all working~~ (oops, test is broken), thank you @imsnif ! :-)

A few more glitchy things fixed, and now viola, actual proof of concept:

simplescreenrecorder-2024-08-29_15.46.26.mp4

imsnif · 2024-08-30T06:46:44Z

Very cool @AutumnMeowMeow !! Just a heads up: I'll be on vacation the next two weeks. Will be sure to answer any questions that come up when I get back.

imsnif · 2024-12-02T14:17:37Z

Hey @AutumnMeowMeow - I hope you are well and don't mind the mention. I want to get this merged and released in the next version, as it will complement some of the planned new features very nicely. Is this something you'd like to keep working on? (no pressure - there is no release date for now)

darrenburns · 2024-12-19T12:38:03Z

Very cool stuff here! Looking forward to this 🙂

* fix(plugins): various cwd fixes * fix tests

* feat(plugins): rerun_command_pane API * fix tests

* separate saved/runtime structure, kind of working * serializing config * work * work * save config through the configuration screen * work * startup wizard * style(code): cleanups * fix(session): reload config from disk when switching sessions * style(fmt): rustfmt * fix(config): propagate cli config options to screen * style(fmt): rustfmt

* feat(ux): reload config at runtime * style(fmt): rustfmt

The response to the 2026 mode query was missing a `?` character. The response should be of the format `CSI ? 2026 ; N $ y` where N can be any value in the range 0-4 inclusive. References: https://gist.github.com/christianparpart/d8a62cc1ab659194337d73e399004036 https://vt100.net/docs/vt510-rm/DECRPM.html

imsnif · 2024-12-31T13:00:37Z

After receiving permission from @AutumnMeowMeow - I'm continuing the work on this. Just rebased from main.

imsnif · 2025-01-09T17:24:55Z

I spent some time with this and at this point almost everything is working:

Copying text with the mouse is working
Dragging floating panes with the mouse is working
Scrolling focused/unfocused panes with the mouse is working
Pinning panes with the mouse is working
Focusing panes with left/right click is working
When Zellij does not intercept the event for one of the above, we serialize it to the pane properly (thanks to @AutumnMeowMeow 's fantastic work)

Left to do functionality-wise (mostly minor quirks):

do not clear the "copied to clipboard" message when the mouse just moves without a button press
text selection doesn't release properly when release event happens outside of pane (probably need to turn self.selecting into an Option to solve this)
if we focus a pane with a right click or middle click, we shouldn't also send the pane the right click
marking text under and left to a pinned pane doesn't work

Otherwise I'd like to get the tests/formatting working, write some more tests for this, do more robust testing for forwarding these events to the underlying panes (maybe vttest as @AutumnMeowMeow suggested) since I might have inadvertently broken something, and then we can merge so we can start adding really cool UI/UX stuff with these crazy good bits of functionality.

…hanged

imsnif · 2025-01-13T17:20:00Z

Right, so - worked on this some more. Everything is mostly fixed and working!

More things to do:
1.~~Fix one e2e test~~ (EDIT: my bad, it's not broken)
2. Adjust the default plugins for the change in API (I decided to issue a breaking behavior change here rather than trying to adapt to the old events) so that stuff like clicking on tab names to focus them will work again
3. Conditionally send a notification to plugins about the state change when clicking a mouse action changed something (we used to do this on every click but this would be quite spammy now!)
4. Write some integration tests for AnyEvent

All in all - not a lot more to do. I'm hoping to have this merged soon.

imsnif · 2025-01-14T14:25:45Z

Everything is done and working great! 🎉

Once the CI passes I'm going to merge this. If @AutumnMeowMeow has the time and feels like testing this, it would be most welcome (even after the merge - three's still some time before the release).

Thanks again for all your hard work!

jondo2010 · 2025-01-17T15:40:34Z

Thank you, been waiting for this since discovering Zellij a couple years ago already 😄

Switch to multi-valued mouse buttons and stub for remembering old mou…

00bf961

…se button state

AutumnMeowMeow temporarily deployed to cachix August 3, 2024 15:41 — with GitHub Actions Inactive

imsnif reviewed Aug 5, 2024

View reviewed changes

AutumnMeowMeow marked this pull request as draft August 6, 2024 20:11

AutumnMeowMeow had a problem deploying to cachix August 6, 2024 20:11 — with GitHub Actions Failure

AutumnMeowMeow commented Aug 6, 2024

View reviewed changes

'cargo xtask build' working using refactored functions.

774241a

AutumnMeowMeow had a problem deploying to cachix August 29, 2024 20:56 — with GitHub Actions Failure

imsnif added 11 commits December 31, 2024 12:28

fix(plugins): various cwd fixes (#3545)

7f3b521

* fix(plugins): various cwd fixes * fix tests

docs(changelog): floating_panes cwd fix

56c6366

feat(plugins): rerun_command_pane API (#3546)

d8875b1

* feat(plugins): rerun_command_pane API * fix tests

docs(changelog): rerun command pane plugin API

afd2645

feat(plugins): command pane re-run event (#3553)

8c1c485

docs(changelog): CommandPaneReRun plugin event

6bdf0e4

docs(changelog): first run setup wizard

bbc750a

feat(ux): reload config at runtime (#3558)

fe5134f

* feat(ux): reload config at runtime * style(fmt): rustfmt

docs(changelog): reload config at runtime

d1cc817

feat(ux): change themes at runtime (#3559)

fe2d46c

darrenburns and others added 5 commits December 31, 2024 12:36

docs(changelog): synchronized rendering query response

7ea8d02

fix(terminal): cursor overflow (#3894)

4210196

docs(changelog): cursor overflow fix

008749e

Merge branch 'main' into AutumnMeowMeow-mouse-anyevent

0d5c8ce

imsnif had a problem deploying to cachix December 31, 2024 12:53 — with GitHub Actions Failure

fix(ux): make the Zellij mouse interaction work

f7a5115

imsnif had a problem deploying to cachix January 9, 2025 17:19 — with GitHub Actions Failure

imsnif added 6 commits January 11, 2025 10:55

fix(rendering): only render if selection/floating-pane position was c…

3432226

…hanged

do not clear copied to clipboard message on mouse motion

a90e9a9

various functionality fixes

3eb026d

fix tests

e6f7a24

fixes and cleanups

b43e7c7

style(fmt): rustfmt

549dd03

imsnif temporarily deployed to cachix January 13, 2025 17:12 — with GitHub Actions Inactive

imsnif added 4 commits January 14, 2025 09:59

fix(mouse): only report state to plugins when it changed

e32e4da

fix(plugins): send mouse clicks to inactive panes

f8ee0f3

tests: any event tracking in panes

c192cba

style(fmt): rustfmt

bf83a51

imsnif temporarily deployed to cachix January 14, 2025 13:53 — with GitHub Actions Inactive

style: remove unused stuff

6d2b8eb

imsnif marked this pull request as ready for review January 14, 2025 14:13

imsnif temporarily deployed to cachix January 14, 2025 14:23 — with GitHub Actions Inactive

imsnif merged commit 1ca7477 into zellij-org:main Jan 14, 2025

AutumnMeowMeow deleted the mouse-anyevent branch January 16, 2025 17:56

Uh oh!

Conversation

AutumnMeowMeow commented Aug 3, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

AutumnMeowMeow commented Aug 3, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

imsnif commented Aug 5, 2024

Uh oh!

imsnif Aug 5, 2024

Choose a reason for hiding this comment

Uh oh!

AutumnMeowMeow commented Aug 6, 2024

Uh oh!

AutumnMeowMeow Aug 6, 2024

Choose a reason for hiding this comment

Uh oh!

AutumnMeowMeow Aug 6, 2024

Choose a reason for hiding this comment

Uh oh!

AutumnMeowMeow Aug 6, 2024

Choose a reason for hiding this comment

Uh oh!

imsnif Aug 8, 2024

Choose a reason for hiding this comment

Uh oh!

imsnif commented Aug 8, 2024

Uh oh!

AutumnMeowMeow commented Aug 29, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

imsnif commented Aug 30, 2024

Uh oh!

imsnif commented Dec 2, 2024

Uh oh!

darrenburns commented Dec 19, 2024

Uh oh!

imsnif commented Dec 31, 2024

Uh oh!

imsnif commented Jan 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

imsnif commented Jan 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

imsnif commented Jan 14, 2025

Uh oh!

jondo2010 commented Jan 17, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

20 participants

AutumnMeowMeow commented Aug 3, 2024 •

edited

Loading

AutumnMeowMeow commented Aug 3, 2024 •

edited

Loading

AutumnMeowMeow commented Aug 29, 2024 •

edited

Loading

imsnif commented Jan 9, 2025 •

edited

Loading

imsnif commented Jan 13, 2025 •

edited

Loading