Add support for OSC 22 control sequence to change mouse cursor shape #6292

reykjalin · 2024-10-18T03:44:00Z

Description

Adds handling for the OSC 22 control sequence.
Follows the example set by Kitty to support CSS cursor values for the OSC 22 parameter.
Currently only supports pointer, text, row-resize, ns-resize, col-resize, and ew-resize, based on the currently available MouseCursor options in the codebase. Any other value defaults to MouseCursor::Arrow.

Videos

Running comlink (terminal IRC client) before and after:

Before (`9ddca7b`)	After
Screen.Recording.2024-10-17.at.11.39.06.PM.mov	Screen.Recording.2024-10-22.at.2.31.45.PM.mov

Testing

This requires a terminal program that makes use of OSC 22 codes to change the mouse cursor. The only one I know of and actively use is comlink, but I'm sure there are other TUIs out there that do use the codes, I'm just not aware of them.

Run a terminal program that uses OSC 22 codes and check if they work.

I'll try to make a terminal program to demonstrate at least some of them as I work on this PR. It should make testing easier.

term/src/terminalstate/mod.rs

wezterm-client/src/pane/clientpane.rs

wezterm-gui/src/overlay/copy.rs

wezterm-gui/src/overlay/quickselect.rs

wezterm-gui/src/termwindow/mod.rs

wezterm-gui/src/termwindow/mouseevent.rs

wez

I'm surprised that you got to this so soon!
A few suggestions below!

mux/src/pane.rs

term/src/terminalstate/mod.rs

term/src/terminalstate/performer.rs

wezterm-gui/src/termwindow/mod.rs

wezterm-gui/src/termwindow/mouseevent.rs

window/Cargo.toml

…optional string

reykjalin · 2024-10-19T05:34:48Z

I'm surprised that you got to this so soon!

You and me both 😅

I think I've addressed most of the feedback, but I'm still seeing the issue where it's like the mouse shape is cached. I'll dig into that some more next week :)

wezterm-gui/src/termwindow/mod.rs

wezterm-gui/src/termwindow/mouseevent.rs

reykjalin · 2024-10-21T02:56:13Z

I think I figured out the reason the mouse cursor shape gets "stuck" on whatever was last set through the OSC 22 sequence. It's because the TUI doesn't set the OSC22 code back to the default, or doesn't clear it I guess? So once the TUI is closed whichever pointer was active is still stored in mouse_cursor_shape in the termstate, so it keeps getting applied over everything else.

I'm not sure if that's "correct" behavior or not. I also don't know what the right time to clear the OSC 22 setting would be, if that even is the correct approach here. The xterm docs don't seem to specify that, at least not in relation to what the control sequence does.

I'll keep looking into this over the next couple of days.

reykjalin · 2024-10-22T18:38:23Z

wezterm-gui/src/termwindow/mouseevent.rs

+        } else if let Some(shape) = pane.get_mouse_cursor_shape() {
+            if pane.is_mouse_grabbed() {
+                parse_mouse_cursor_shape(shape.as_str())
+            } else {
+                // If the mouse is no longer grabbed by a TUI, reset the cursor shape.
+                pane.clear_mouse_cursor_shape();
+                MouseCursor::Arrow
+            }
+        } else if pane.is_mouse_grabbed() {


As it turns out, this fixed the issue of the mouse cursor shape being wrong. I have absolutely no idea if this is the right way to approach it though.

But basically; if the mouse is grabbed by whichever application is running, we defer to whatever mouse shape is set by any OSC 22 escape sequences.

If we find that a shape has been set by an OSC 22 escape sequence but the mouse isn't grabbed we clear it and just use the default pointer shape instead.

Some thoughts:

I'm realizing that using mouse_cursor_shape for the names of this might be confusing? Should we specify that this is a shape set through an OSC 22 escape sequence somehow?

It's more confusing in the GUI layer, less so in the terminal state itself.

But it may be useful to use something like osc_22_mouse_cursor_shape instead of just mouse_cursor_shape?

I'm not sure this is the best place to clear the mouse cursor shape. Maybe there's an event sent when the mouse is grabbed and released that would be more appropriate?

As stated earlier; I have no clue if this is the right way to manage OSC 22 control sequences, but at least it behaves the way I expect.

Before this change After this change

Screen.Recording.2024-10-22.at.2.36.17.PM.mov

Screen.Recording.2024-10-22.at.2.31.45.PM.mov

Happy to get some guidance on any improvements here!

I think we should implicitly clear the cursor shape when the mouse is un-grabbed, then the original gating logic:

else if pane.is_mouse_grabbed() || outside_window { MouseCursor::Arrow

can be used there to very clearly show that we're not going to use the cursor shape in those conditions.

Each of these locations are places where the effective result is is_mouse_grabbed might change to false:

wezterm/term/src/terminalstate/mod.rs

Lines 1764 to 1767 in c06cc87

Mode::ResetDecPrivateMode(DecPrivateMode::Code(DecPrivateModeCode::MouseTracking)) => {

self.mouse_tracking = false;

self.last_mouse_move.take();

}

wezterm/term/src/terminalstate/mod.rs

Lines 1799 to 1802 in c06cc87

Mode::ResetDecPrivateMode(DecPrivateMode::Code(DecPrivateModeCode::AnyEventMouse)) => {

self.any_event_mouse = false;

self.last_mouse_move.take();

}

wezterm/term/src/terminalstate/mod.rs

Lines 1783 to 1788 in c06cc87

Mode::ResetDecPrivateMode(DecPrivateMode::Code(

DecPrivateModeCode::ButtonEventMouse,

)) => {

self.button_event_mouse = false;

self.last_mouse_move.take();

}

There should probably be a little helper function called in those three cases that does something like:

if !self.is_mouse_grabbed() && self.mouse_cursor_shape.is_some() { self.mouse_cursor_shape.take(); mouse_cursor_shape_did_change }

with all that done, the logic here in this file should simplify a lot and be very clear and easy to follow.

FWIW, I think mouse_cursor_shape is OK to refer to this stuff.

reykjalin · 2024-11-30T00:22:31Z

Hi @wez 👋 Just wanted to ask if there's anything I can do to make moving this along easier?

I think the changes here are at least functionally good, so I'd love to help move this along if there's anything I can do to help with that :)

wez

Thanks for this!
A few comments here.

You should also add OSC 22 to https://wezterm.org/escape-sequences.html so that this support is discoverable.

wez · 2025-02-11T18:32:25Z

mux/src/pane.rs

@@ -231,6 +231,9 @@ pub trait Pane: Downcast + Send + Sync {
    /// Returns render related dimensions
    fn get_dimensions(&self) -> RenderableDimensions;

+    fn get_mouse_cursor_shape(&self) -> Option<String>;
+    fn clear_mouse_cursor_shape(&self);


IMO, clear_mouse_cursor_shape doesn't belong in this interface.
The terminal itself owns the current shape, and the Pane should just report its state.
I don't think it makes sense for other layers to poke it, which is good because that is one less RPC call that will need to be added to the mux protocol to accommodate this change.

wez · 2025-02-11T18:32:39Z

mux/src/pane.rs

@@ -550,6 +553,14 @@ mod test {
    }

    impl Pane for FakePane {
+        fn get_mouse_cursor_shape(&self) -> Option<String> {
+            unimplemented!()


Suggested change

unimplemented!()

None

wez · 2025-02-11T18:32:48Z

mux/src/tab.rs

@@ -2222,6 +2222,14 @@ mod test {
    }

    impl Pane for FakePane {
+        fn get_mouse_cursor_shape(&self) -> Option<String> {
+            unimplemented!();


Suggested change

unimplemented!();

None

wez · 2025-02-11T18:34:59Z

term/src/terminal.rs

@@ -51,6 +51,8 @@ pub enum Alert {
    TabTitleChanged(Option<String>),
    /// When the color palette has been updated
    PaletteChanged,
+    /// When the mouse cursor shape has been updated
+    MouseCursorShapeChanged,


Let's pass the name along with this, like with the title changed event above.
That will make it easier to do a half-baked mux client implementation (see 44866cc)

Suggested change

MouseCursorShapeChanged,

MouseCursorShapeChanged(Option<String>)

Note that by changing the Alert enum, you're changing the binary signature of types on which the mux protocol depends. You'll need to bump the CODEC_VERSION:

wezterm/codec/src/lib.rs

Lines 441 to 444 in c06cc87

/// The overall version of the codec.

/// This must be bumped when backwards incompatible changes

/// are made to the types and protocol.

pub const CODEC_VERSION: usize = 44;

wez · 2025-02-11T18:35:42Z

term/src/terminalstate/mod.rs

+    /// Clears the mouse cursor shape.
+    pub fn clear_mosue_cursor_shape(&mut self) {
+        self.mouse_cursor_shape = None;
+    }
+


as mentioned elsewhere, I think this should be removed from the interface

Suggested change

/// Clears the mouse cursor shape.

pub fn clear_mosue_cursor_shape(&mut self) {

self.mouse_cursor_shape = None;

}

wez · 2025-02-11T18:46:56Z

wezterm-gui/src/termwindow/mouseevent.rs

+        } else if let Some(shape) = pane.get_mouse_cursor_shape() {
+            if pane.is_mouse_grabbed() {
+                parse_mouse_cursor_shape(shape.as_str())
+            } else {
+                // If the mouse is no longer grabbed by a TUI, reset the cursor shape.
+                pane.clear_mouse_cursor_shape();
+                MouseCursor::Arrow
+            }
+        } else if pane.is_mouse_grabbed() {


I think we should implicitly clear the cursor shape when the mouse is un-grabbed, then the original gating logic:

else if pane.is_mouse_grabbed() || outside_window { MouseCursor::Arrow

can be used there to very clearly show that we're not going to use the cursor shape in those conditions.

Each of these locations are places where the effective result is is_mouse_grabbed might change to false:

wezterm/term/src/terminalstate/mod.rs

Lines 1764 to 1767 in c06cc87

Mode::ResetDecPrivateMode(DecPrivateMode::Code(DecPrivateModeCode::MouseTracking)) => {

self.mouse_tracking = false;

self.last_mouse_move.take();

}

wezterm/term/src/terminalstate/mod.rs

Lines 1799 to 1802 in c06cc87

Mode::ResetDecPrivateMode(DecPrivateMode::Code(DecPrivateModeCode::AnyEventMouse)) => {

self.any_event_mouse = false;

self.last_mouse_move.take();

}

wezterm/term/src/terminalstate/mod.rs

Lines 1783 to 1788 in c06cc87

Mode::ResetDecPrivateMode(DecPrivateMode::Code(

DecPrivateModeCode::ButtonEventMouse,

)) => {

self.button_event_mouse = false;

self.last_mouse_move.take();

}

There should probably be a little helper function called in those three cases that does something like:

if !self.is_mouse_grabbed() && self.mouse_cursor_shape.is_some() { self.mouse_cursor_shape.take(); mouse_cursor_shape_did_change }

with all that done, the logic here in this file should simplify a lot and be very clear and easy to follow.

FWIW, I think mouse_cursor_shape is OK to refer to this stuff.

wez · 2025-02-11T18:50:27Z

wezterm-client/src/pane/clientpane.rs

@@ -245,6 +245,12 @@ impl ClientPane {

 #[async_trait(?Send)]
 impl Pane for ClientPane {
+    fn get_mouse_cursor_shape(&self) -> Option<String> {


Take a look at the changes over in 44866cc#diff-b8703182c50d1b02b59221d8680fa9e6ec9eeb8520beb8e34d85f933fca7fac5R195 for how to do an imperfect but probably good enough in practice implementation of this that will work over the mux protocol

reykjalin · 2025-02-11T20:51:22Z

Thanks for the review Wez! Just want to let you know this is on my radar, but I'm going to be busy until the end of March so I might not be able to address the review until then.

I'll follow up as soon as I'm able to! :)

reykjalin added 2 commits October 17, 2024 23:23

Add flaky-ish support for OSC 22

278dfdd

fix failing tests

20ce388

reykjalin changed the title ~~Add support for OSC 22 control sequence to change mouse shape~~ Add support for OSC 22 control sequence to change mouse cursor shape Oct 18, 2024

remove commented out code

9587cf6

reykjalin commented Oct 18, 2024

View reviewed changes

wezterm-gui/src/termwindow/mouseevent.rs Outdated Show resolved Hide resolved

wez reviewed Oct 18, 2024

View reviewed changes

reykjalin added 2 commits October 19, 2024 01:13

move MouseCursor back to window and make mouse cursor shape state an …

b67cf84

…optional string

update mouse shape parsing function

683d134

wez reviewed Oct 19, 2024

View reviewed changes

wezterm-gui/src/termwindow/mod.rs Outdated Show resolved Hide resolved

wezterm-gui/src/termwindow/mouseevent.rs Outdated Show resolved Hide resolved

reykjalin added 2 commits October 20, 2024 22:28

make parse_mouse_cursor_shape a freestanding function

0cd76fc

allow OSC22 code to override when mouse is grabbed

11cf8b6

Fix wrong mouse cursor shape after it's been set with OSC 22 sequence

2251204

reykjalin commented Oct 22, 2024

View reviewed changes

reykjalin marked this pull request as ready for review October 22, 2024 18:39

add tests for parsing osc 22 codes

94b5acb

reykjalin requested a review from wez November 1, 2024 03:04

wez reviewed Feb 11, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for OSC 22 control sequence to change mouse cursor shape #6292

Add support for OSC 22 control sequence to change mouse cursor shape #6292

reykjalin commented Oct 18, 2024 •

edited

Loading

wez left a comment

reykjalin commented Oct 19, 2024

reykjalin commented Oct 21, 2024 •

edited

Loading

reykjalin Oct 22, 2024 •

edited

Loading

wez Feb 11, 2025

reykjalin commented Nov 30, 2024

wez left a comment

wez Feb 11, 2025

wez Feb 11, 2025

wez Feb 11, 2025

wez Feb 11, 2025

wez Feb 11, 2025

wez Feb 11, 2025

wez Feb 11, 2025

wez Feb 11, 2025

reykjalin commented Feb 11, 2025

	Mode::ResetDecPrivateMode(DecPrivateMode::Code(DecPrivateModeCode::MouseTracking)) => {
	self.mouse_tracking = false;
	self.last_mouse_move.take();
	}

	Mode::ResetDecPrivateMode(DecPrivateMode::Code(DecPrivateModeCode::AnyEventMouse)) => {
	self.any_event_mouse = false;
	self.last_mouse_move.take();
	}

	Mode::ResetDecPrivateMode(DecPrivateMode::Code(
	DecPrivateModeCode::ButtonEventMouse,
	)) => {
	self.button_event_mouse = false;
	self.last_mouse_move.take();
	}

	MouseCursorShapeChanged,
	MouseCursorShapeChanged(Option<String>)

	/// The overall version of the codec.
	/// This must be bumped when backwards incompatible changes
	/// are made to the types and protocol.
	pub const CODEC_VERSION: usize = 44;

Add support for OSC 22 control sequence to change mouse cursor shape #6292

Are you sure you want to change the base?

Add support for OSC 22 control sequence to change mouse cursor shape #6292

Conversation

reykjalin commented Oct 18, 2024 • edited Loading

Description

Videos

Testing

wez left a comment

Choose a reason for hiding this comment

reykjalin commented Oct 19, 2024

reykjalin commented Oct 21, 2024 • edited Loading

reykjalin Oct 22, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

reykjalin commented Nov 30, 2024

wez 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

reykjalin commented Feb 11, 2025

reykjalin commented Oct 18, 2024 •

edited

Loading

reykjalin commented Oct 21, 2024 •

edited

Loading

reykjalin Oct 22, 2024 •

edited

Loading