Change Enable/Diable_MouseCapture to be more granular

[sigmaSd]

trafficstars

fix https://github.com/crossterm-rs/crossterm/issues/640

[sigmaSd]

Windows is currently hardcoded to use the winapi which doesn't seem to offer this level of control according to https://docs.microsoft.com/en-us/windows/console/setconsolemode

What if we make windows use ansi as well, maybe it understand these codes?

[markus-bauer]

First of all, thank you for the quick response and making this pull request.

I did a quick test (not extensive) with several linux terminals, and it works as expected.

Some observations:

1. Should the more esoteric options be exposed? I wouldn't know what mouse mode I should enable. Rxvt, SGR, both or neither?
2. This breaks backwards compatibility: I think the commands EnableMouseCapture and DisableMouseCapture should stay as they are and call something like EnableMouseCaptureOptions::default().
3. While testing this, it seemed as if MouseTrackingMode::Any doesn't just "Report all motion events" but all events (motion, drag and click). This would mean that there really are only three options that the user might want: any, drag (button+motion), button. So the interface could be: EnableMouseCapture (=EnableMouseCaptureAll), EnableMouseCaptureButton, EnableMouseCaptureDrag. This is obviously only true if my assumption about Any is true.
4. write!(f, "{}", mode.enable() can be replaced with f.write_str(mode.enable())

[sigmaSd]

I agree with all these points, for 3 I had the same thoughts If someone can clear the windows situation I'll rewrite it as you suggested

[markus-bauer]

I'm actually not sure about what to expose to the user. Exposing all is more consistent, but the other way is more immediately understandable.

Sorry, I don't know anything about windows. Looking through https://github.com/microsoft/terminal, I found this: https://github.com/microsoft/terminal/issues/10531#issuecomment-872398721

The best way is to test it, but I don't have a windows machine.

Perhaps wait for another opinion, before doing the work of rewriting it.

[sigmaSd]

Here is another take https://github.com/crossterm-rs/crossterm/compare/master...sigmaSd:mouse1?expand=1 @markus-bauer

[markus-bauer]

Yes this also works.

I realized, that there are cases where this might not do what the user thinks it does. For example: EDIT: the following might not be correct, see: https://github.com/crossterm-rs/crossterm/pull/641#issuecomment-1066159668

stdout.execute(MouseCaptureOptions::CaptureButton(true));
// This does not disable all events:
stdout.execute(MouseCaptureOptions::CaptureAny(false));

// Only after this are button events disabled:
stdout.execute(MouseCaptureOptions::CaptureButton(false));

This is just a thought: CaptureAny(false) could disable everything (Any, Drag, Button). Similarly, CaptureDrag(false) could disable Drag and Button. Otherwise, this has to be documented properly.

---

I would like to know the opinion of @TimonPost, before continuing:

1. What should be done about the windows code?

2. Would you merge this, if it can't be made cross platform?

3. Should this expose all options or should it be the three options we mentioned?

4. What should the interface be like? The proposals by @sigmaSd so far were:

   a)

   EnableMouseCapture([MouseTrackingMode::Normal, ...])
   DisableMouseCapture([MouseTrackingMode::Normal, ...])
   

   b)

   // enable:
   MouseCaptureOptions::CaptureButton(true)
   // disable:
   MouseCaptureOptions::CaptureButton(false)
   
   // and similarly for other options:
   MouseCaptureOptions::CaptureAny(true)
   ...
   

   c) I would suggest: If there are only 3 options, then there could be 3 separate commands:

   stdout.execute(EnableMouseCapture)
   stdout.execute(DisableMouseCapture)
   
   stdout.execute(EnableMouseCaptureClick)
   stdout.execute(DisableMouseCaptureClick)
   
   stdout.execute(EnableMouseCaptureDrag)
   stdout.execute(DisableMouseCaptureDrag)
   

[sigmaSd]

Regarding windows, recent versions support ansi well, it could be it just works, if someone wants to test you need to use this branch https://github.com/crossterm-rs/crossterm/compare/master...sigmaSd:mouse1?expand=1

[TimonPost]

We could hide the implementation behind a target platform feature flag. But that will make the code break if you switch to windows. Alternatively, we could also just do nothing, and add in the comments that the behavior will only properly work on UNIX. Defaulting to just passing on all events seem reasonable to me, at least it doesn't break the API. Currently, key combinations work the same way, windows support way more than Unix.

[TimonPost]

I would prefer this API:

EnableMouseCapture([MouseTrackingMode::Normal, ...])
DisableMouseCapture([MouseTrackingMode::Normal, ...])

or maybe with the bitmask we can do:

EnableMouseCapture(MouseTrackingMode::Normal | MouseTrackingMode::Other)
DisableMouseCapture(MouseTrackingMode::Normal, ...)

[markus-bauer]

But what do you think the actual goal of this should be? a) Make it possible for the user to set/unset all mouse flags. b) Set one of three modes (Move+Drag+Click, Drag+Click, Click) and handle the rest under the hood.

[TimonPost]

I would also not know when to enable Rxvt or sgr. It seems cleaner if the user can just combine move, drag, click. But I am also not sure if we can detect if underlying we should use rxvt or sgr?

[markus-bauer]

As I mentioned above, I don't thinks this is not how it works. They seem to be distinct modes.

If you enable Button-event tracking, then it sends events for Drag and Button (and Scroll). So you don't need to combine them.

This has a description of the modes: https://invisible-island.net/xterm/ctlseqs/ctlseqs.html#h2-Mouse-Tracking

---

About the interface: There could be a low level and high level interface. One to set the flags you want, and one that has 3 specific modes.

[markus-bauer]

Quick correction: Before, I was testing in gnome-terminal, but in xterm I get different results:

stdout.execute(MouseCaptureOptions::CaptureAny(true));
// this turns off *all* mouse events:
stdout.execute(MouseCaptureOptions::CaptureButton(false));

stdout.execute(MouseCaptureOptions::CaptureAny(true));
// this overwrites Any. Now only Button events are reported:
stdout.execute(MouseCaptureOptions::CaptureButton(true));

If this is true, then these modes don't even combine. And this would mean, that the existing code for EnableMouseCapture doesn't work as it seems, because the later modes would overwrite the earlier ones. You can test it by changing the sequence in EnableMouseCapture.

I don't have time to do more testing today. I will look into it next week.

[markus-bauer]

Here are some tests (This is a long one):

Setup

• This uses the original pull request (including the recent fixes), because you can enable/disable the modes individually and as a sequence.

• System: Ubuntu 21.10

• Terminal Emulators:

  • G: GNOME Terminal 3.38.1 using VTE 0.64.2 +BIDI +GNUTLS +ICU +SYSTEMD
  • X: XTerm(366)
  • K: kitty 0.24.2
  • R: rxvt-unicode (urxvt) v9.22

• Terminal Emulators were newly started for every run.

• Mouse events are defined as:

  • Move
  • Button (incl. Scroll)
  • Drag
  • All = Move + Button + Drag.
  • None = no mouse events

• Program used for testing (code at the end of this post):

  1. enable a sequence of mouse modes
  2. optionally disable some mouse modes
  3. get events in a loop. The following results show which kind of events this sends.
  4. at the end: disable the mouse modes

---

Results

a) Enable(Any), loop, Disable(Any):

• G: All
• X: All
• K: All
• R: All

b) Enable(Normal), loop, Disable(Normal):

• G: Button
• X: Button
• K: Button
• R: Button

c) Enable(Any, Normal), loop, Disable(Any, Normal):

• G: All
• X: Button
• K: Button
• R: Button

d) Enable(Normal, Any, ButtonEvent), loop, Disable(Normal, Any, ButtonEvent):

• G: All
• X: Drag + Button
• K: Drag + Button
• R: Drag + Button

e) Enable(Normal, Any), loop, Disable(Normal, Any):

• G: All
• X: All
• K: All
• R: All

f) Enable(Normal, Any), Disable(Normal), loop, Disable(Any)

• G: All
• X: None
• K: None
• R: All

g) Enable(Normal, Any), Disable(Any), loop, Disable(Normal)

• G: Button
• X: None
• K: None
• R: None

h) Enable(Any, Normal), Disable(Normal), loop, Disable(Any)

• G: All
• X: None
• K: None
• R: None

i) Enable(Any, Normal), Disable(Any), loop, Disable(Normal)

• G: Button
• X: None
• K: None
• R: Button

j) Enable(Normal), Disable(Any), loop, Disable(Normal)

• G: Button
• X: None
• K: None
• R: Button

k) Enable(Any), Disable(Normal), loop, Disable(Any)

• G: All
• X: None
• K: None
• R: All

---

Can someone please reproduce/confirm at least some of the tests. And try other terminals.

---

Some observations:

• The bug in the original pull request showed, that enabling and disabling has to be symmetric, otherwise it will leave the terminal in a broken state.
• Combining modes can lead to unexpected and unpredictable behaviour and seems to be dependent on the implementation of the terminal emulator.
• Combining modes is not necessary, since they encompass each other:
  • Normal = Button
  • ButtonEvent = Drag + Button
  • Any = Move + Drag + Button
• Modes are distinct. You could assume the following, but it isn't true: Enable(Any) + Disable(ButtonEvent) = (Move + Drag + Button) - (Button + Drag) = Move

---

Example program used for testing:

use crossterm::{
    cursor::MoveTo,
    event::{
        poll, read, DisableMouseCapture, EnableMouseCapture, Event, KeyCode, KeyEvent,
        MouseEventKind, MouseTrackingMode,
    },
    style::Print,
    terminal::{self, Clear, ClearType, EnterAlternateScreen, LeaveAlternateScreen},
    ExecutableCommand, Result,
};
use std::io::stdout;
use std::time::Duration;

fn main() -> Result<()> {
    let mut stdout = stdout();
    terminal::enable_raw_mode()?;
    stdout.execute(EnterAlternateScreen)?;

    // -- parameter:
    stdout.execute(EnableMouseCapture([
        MouseTrackingMode::Any,
        MouseTrackingMode::Normal,
    ]))?;

    // -- parameter:
    stdout.execute(DisableMouseCapture([MouseTrackingMode::Any]))?;

    loop {
        if poll(Duration::from_millis(500)).unwrap() {
            match read().unwrap() {
                Event::Key(KeyEvent {
                    code: KeyCode::Char('q'),
                    ..
                }) => break,
                Event::Mouse(event) => {
                    stdout.execute(MoveTo(0, 0))?;
                    match event.kind {
                        MouseEventKind::Up(..) => stdout.execute(Print("up"))?,
                        MouseEventKind::Down(..) => stdout.execute(Print("down"))?,
                        MouseEventKind::ScrollUp => stdout.execute(Print("scroll up"))?,
                        MouseEventKind::ScrollDown => stdout.execute(Print("scroll down"))?,
                        MouseEventKind::Drag(..) => stdout.execute(Print("drag"))?,
                        MouseEventKind::Moved => stdout.execute(Print("move"))?,
                    };
                    stdout.execute(Clear(ClearType::UntilNewLine))?;
                }
                _ => (),
            }
        }
    }

    // -- parameter:
    stdout.execute(DisableMouseCapture([MouseTrackingMode::Normal]))?;

    terminal::disable_raw_mode()?;
    stdout.execute(LeaveAlternateScreen)?;

    Ok(())
}

[markus-bauer]

I probably won't contribute to this PR anymore. Most has been said and the final decisions are not mine to make. My thoughts on the situation:

1. We should keep EnableMouseCapture/DisableMouseCapture as they are. The main reason is, that we should not mess with backwards compatibility. This should be a high level command to just enable/disble all mouse functionality. Although the unix code can perhaps be simplified (we might not need to set all modes).
2. There should be a new low level api that lets the user set any mouse mode they want. It will be on the user to not break things. This should be available on all platforms: If someone on windows wants to use xterm codes, why shouldn't we let them? There's always the option to fall back to EnableMouseCapture
3. Based on this, I think the interface for the low level api I like the most is this:

// multiple
EnableMouseModes([MouseMode::Any, MouseMode::Rxvt])
DisableMouseModes([MouseMode::Any, MouseMode::Rxvt])

// optional: single mode
EnableMouseMode(MouseMode::Any)
DisableMouseMode(MouseMode::Any)

If my tests are correct, then it might be relevant in what order the Modes are set: Enable(Any, Normal) might not give the same results as Enable(Normal, Any). So flags might not work. But I'm not sure about this. 4) Additionally to 1), we could introduce new high level Commands. If EnableMouseCapture boils down to setting MouseMode::Any (+ SGR/RXVT), then there could be Commands for the other two modes: EnableMouseCaptureButton = MouseMode::Normal (+...) and EnableMouseCaptureDrag = MouseMode::ButtonEvent (+...) Note: these might not be cross platform.

---

Summary:

1. low level api:

EnableMouseModes([MouseMode::Any, MouseMode::Rxvt])
DisableMouseModes(MouseMode::Any, ...])

2. new high level Commands: EnableMouseCaptureButton/DisableMouseCaptureButton and EnableMouseCaptureDrag/DisableMouseCaptureDrag
3. perhaps: simplify unix code of EnableMouseCapture

[TimonPost]

Sorry for being idle. A question I would have: would users frequently switch between modes, enabling some, disabling others etc... Now I'm not a CLI app developer, but I can imagine that you would want to enable inputs in a certain way, and then leave it that way for the duration of the program.

Reading the previous comments, it seems like the behavior of those flags is very unpredictable for different platforms. If this is true, I think we could just expose everything and let the users configure it as they wish. This is also in line with crossterm's vision, not to keep too much state and internal logic, and let users decide how to implement the behavior with it. Yes, this might make the API a bit more complicated for this particular case, but I think we can just add a comment with a recommendation like 'enable all mode'. When users start experimenting with enabling and disabling features, it's up to them to make it work. And we'll just comment that it may have unexpected results.

[TimonPost]

Thus, agreeing with the 1:

EnableMouseModes([MouseMode::Any, MouseMode::Rxvt]) DisableMouseModes(MouseMode::Any, ...])

This to mee seems to make most sense, as playing with combinations is upto the users while sparing us the pain to internally manage allowed calls, order of those calls, and platform specific restrictions. Thus, this PR is on the right track working towards this. Other library devs could implement a state machine of some kind if they are upto it.

[markus-bauer]

I agree with everything you said. The way to go is version 1 (what you said). In fact, this is what this PR already is.

About switching modes: In my program, I would enable the modes I want at the start, and disable the same ones at the end. I don't think switching modes would be useful or even recommended.

The weird thing is: It seems that combining modes doesn't always do what you expect and the behavior might be up to the terminal. And, combining modes might not be necessary. Then the user might ask: Then why should I pass several modes to EnableMouseModes? You certainly have to document this feature properly.

---

But what do you think about the rest? Did I explain the situation properly?

1. If my tests were correct, then there might not be such a thing as enabling all modes at once (like the current version of EnableMouseCapture thinks it is doing). Take this example from before:

Enable(Normal, Any, ButtonEvent):
    G: All
    X: Drag + Button
    K: Drag + Button
    R: Drag + Button

And a quick test I did just now gave this:

Enable(ButtonEvent, Normal):
    G: Button + Drag
    R=X=K: Button

Enable(Any, ButtonEvent, Normal):
    G: All
    R=X=K: Button

So they don't combine. This means EnableMouseCapture just works, because you set Any as the last mode.

2. And if this is true, then EnableMouseCapture will turn into the equivalent of

EnableMouseModes([MouseMode::Any, MouseMode::Rxvt, MouseMode::SGR])

So the other modes (which enable subsets of Any) would be analogous:

EnableMouseCaputureButton = EnableMouseModes([MouseMode::Normal, MouseMode::Rxvt, MouseMode::SGR])
EnableMouseCaputureDrag = EnableMouseModes([MouseMode::ButtonEvent, MouseMode::Rxvt, MouseMode::SGR])

Anyway, this isn't really related to this PR. I just want to mention this for the record.

[TimonPost]

Interesting finding. And nice writeup btw! I dont run a Linux PC so I cant easily validate your results. How would you suggest going on with this? Seems to me there is one good solution right now: Just allow all combinations and write documentation on how to achieve the three most sense-making modes (all, drag, button) while stressing that combining things leads to unexpected behavior across various platforms.

But would it make sense to support combining if combining doest even work well?

[markus-bauer]

About combining:

https://invisible-island.net/xterm/ctlseqs/ctlseqs.html#h2-Mouse-Tracking:

There are two sets of mutually exclusive modes:

o mouse protocol

o protocol encoding

And from https://terminalguide.namepad.de/mouse/:

Configuration is for 2 independent aspects:

• What events are sent and their button encoding.
• Which report encoding is used

...

Events

These modes are mutually exclusive. The last activated mode wins.

That last page explains the situation pretty well.

What I understand from reading this is: There is no combining. There is one mode you set. If you set another one, this will not somehow combine with the previous one. Additionally (and independently) you can set the encoding. I would also assume that only one is set in the terminal at a time.

So, anything beyond [Any, Rxvt] (for example) is probably not needed in practice.

The only thing that is unpredictable is how different terminals react to setting modes in sequence.

[markus-bauer]

But I want to be clear: I know next to nothing about any of this. Before this PR, I didn't even know what xterm mouse modes are. All I personally want is a feature to turn off events for mouse movement. I don't know if the mouse modes are the right solution. I just stumbled on the fact, that different terminals reacted differently to this PR. Then I did the tests. I explained my test, gave the code, and reported the results.

The final version should not be solely based on something I did on my machine, based on no actual knowledge.

[sigmaSd]

@markus-bauer thanks for all the work you have done!

My personal take based on your findings and https://invisible-island.net/xterm/ctlseqs/ctlseqs.html#h2-Mouse-Tracking is to just make the events separate, and enable sgr and rxvt since that's whats already done in the original command.

Regarding your last comment, no one has the full picture and if we wait for someone like that to showup it might take forever. I think a better approach is to start with something and people will use it and report issue and improve it with time. Also this is a new command so its backward compatible so it won't be that much of a problem.

This pr will use ansi on windows if available, I think it should work but I haven't tested it.

[TimonPost]

I was trying out your branch but found some issues on windows in powershell. But none of the mouse options seem to work.

[sigmaSd]

I don't have windows currently to test, hopefully someone else picks this up if its still interesting.