Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix SGR/UTF8 mouse reporting in terminal panes #1664

Merged
merged 12 commits into from
Aug 17, 2022
Merged

Conversation

imsnif
Copy link
Member

@imsnif imsnif commented Aug 14, 2022

This is a fix for: #1633

Main changes:

  1. We now track which mouse button was released when collecting user input (rather than sending a generic "release" event on to the zellij server).
  2. DECSET/DECRST now loop through all arguments rather than just taking the first one (oops!)
  3. Mouse mode is now tracked as "NoEncoding", "SGR" or "UTF8" rather than true/false
  4. We now also track the mouse tracking method as "Off", "Normal" and "ButtonEventTracking", reporting nothing, button press + release and button press + release + hold respectively.
  5. We send the relevant events from 4 with the relevant encoding from 3 to the terminal panes as needed. Defaulting to utf8 encoding.

There's room to implement AnyEvent tracking in supported terminals, but that's out of scope for this PR.

@imsnif imsnif temporarily deployed to cachix August 14, 2022 11:51 Inactive
@imsnif imsnif temporarily deployed to cachix August 14, 2022 11:54 Inactive
@imsnif
Copy link
Member Author

imsnif commented Aug 14, 2022

@tlinford - if you could give this a spin to make sure I didn't break any mouse stuff that I'm not aware of that would be great :)

@AutumnMeowMeow - if you have the time and feel like it, would you like to check this change out and let me know what you think?

@imsnif imsnif temporarily deployed to cachix August 14, 2022 12:07 Inactive
@AutumnMeowMeow
Copy link
Contributor

@imsnif Testing inside vttest, I see the following issues:

  • Off-by-one coordinates: SGR mode (1006), button-event (1002), during a drag-with-button-down: the coordinates are incorrectly +1 rows +1 cols. The button-down and button-up coordinates are correct.
  • Off-by-one coordinates: UTF8 mode (1005), normal-event (1000), during a button-release: the coordinates are incorrectly +1 rows +1 cols. The button-down coordinates are correct.
  • Off-by-one coordinates: UTF8 mode (1005), button-event (1002), during a button-release and during a drag-with-button-down: the coordinates are incorrectly +1 rows +1 cols. The button-down coordinates are correct.
  • When a test is exited via 'q', mouse tracking is still on and not off. You are missing the corresponding cases from here for 1000, 1002, 1003 (TBD) that belong here.

@imsnif imsnif temporarily deployed to cachix August 15, 2022 20:17 Inactive
@imsnif imsnif temporarily deployed to cachix August 15, 2022 20:18 Inactive
@imsnif
Copy link
Member Author

imsnif commented Aug 15, 2022

Thanks @AutumnMeowMeow ! I guess I was pretty tired when I submitted this. I even glanced over these in the tests :) All fixed.

@AutumnMeowMeow
Copy link
Contributor

@imsnif It's better now, but I still see the following:

  • Button 2 (usually middle button) is not reporting on button-down or button-down-and-drag, but is reporting on button-up/release.
  • In SGR coordinates, when button 2 reports button-up, it incorrectly reports it as button 1 (code 0) button-up.
  • In both UTF-8 and SGR coordinates, button-down-and-drag is not being reported as a drag (button code + 32), but rather as button-down (button code).

@imsnif imsnif temporarily deployed to cachix August 16, 2022 13:02 Inactive
@imsnif imsnif temporarily deployed to cachix August 16, 2022 13:03 Inactive
@imsnif
Copy link
Member Author

imsnif commented Aug 16, 2022

@AutumnMeowMeow - all fixed. These were part misunderstanding on my part, and part Zellij not previously fully supporting middle-clicks. Thanks again!

@tlinford
Copy link
Contributor

@imsnif gave it a test, all of these seem fine to me:

  • pane/tab selection with mouse
  • moving floating pane
  • selecting text
  • scrolling with mouse wheel
  • interacting with nvim with set mouse=a

@AutumnMeowMeow
Copy link
Contributor

I confirm normal-event and button-event are working correctly for me:

simplescreenrecorder-2022-08-16_13.46.27.mp4

Any-event is not yet working, so no tracking mouse cursor tracking, but that is expected. Click/drag/release and wheel-up/wheel-down are working well.

I would say #1633 is fixed with this. :)

@imsnif
Copy link
Member Author

imsnif commented Aug 17, 2022

Fantastic! Thank you both for your help.

@imsnif imsnif merged commit f4ad946 into main Aug 17, 2022
@har7an har7an deleted the fix-mouse-reporting branch October 23, 2022 15:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants