[Conpty] Pass through request for mouse mode to the Terminal #9970

PankajBhojwani · 2021-04-28T04:22:43Z

Summary of the Pull Request

When the client application sets the console input mode with the ENABLE_MOUSE_INPUT flag, we send along the appropriate VT sequences to the connected terminal (if there is one).

References

#376
#6859

PR Checklist

Closes #xxx
CLA signed. If not, go over here and sign the CLA
Tests added/passed
Documentation updated. If checked, please file a pull request on our docs repo and link it here: #xxx
Schema updated.
I work here

Validation Steps Performed

Far manager works

PankajBhojwani · 2021-04-28T04:24:29Z

src/host/getset.cpp

+            gci.GetActiveInputBuffer()->PassThroughEnableButtonEventMouseMode(true);
+            gci.GetActiveInputBuffer()->PassThroughEnableSGRExtendedMouseMode(true);


Only sending these 2 VT sequences seems to make far manager work, but I am not sure if we should be sending other VT sequences through as well/in different cases (like only button events versus all mouse events), since the console mode only has 1 umbrella ENABLE_MOUSE_INPUT mode and not more specific ones as far as I know

Can you try if Win32 Vim works? The lacking of mouse support in Win32 Vim is the root cause of several issues. It also drives me to NeoVim personally.

We probably want 1003 instead of 1002 -- that's the "any event" mode which includes hover (the win32 event model automatically reports hover!)

src/host/inputBuffer.hpp

PankajBhojwani · 2021-04-28T04:31:18Z

2 outstanding issues:

Tested this with far manager in terminal and it works fine with cmd, but there's a bug when using powershell: upon exiting far in powershell, for some reason powershell never asks us to set the input mode back to 'no mouse input', so Terminal is stuck in enable_mouse_input mode, breaking things like selection. I am not sure if this is a powershell issue or something that we need to deal with (or both)? This seems to work now and I'm not sure why (somehow the input mode code that powershell was sending changed?)
Ubuntu sets the console's mouse input mode simply upon starting up, so we cannot create a selection in an Ubuntu tab/pane. Not sure if this is specific to Ubuntu or WSL in general

src/host/getset.cpp

skyline75489 · 2021-04-28T06:14:18Z

src/host/getset.cpp

+            gci.GetActiveInputBuffer()->PassThroughEnableButtonEventMouseMode(true);
+            gci.GetActiveInputBuffer()->PassThroughEnableSGRExtendedMouseMode(true);


Can you try if Win32 Vim works? The lacking of mouse support in Win32 Vim is the root cause of several issues. It also drives me to NeoVim personally.

src/host/getset.cpp

…/pabhoj/mouse_mode

src/host/getset.cpp

PankajBhojwani · 2021-05-05T02:47:08Z

The issues with powershell and WSL have been resolved (as far as I can tell), so I've marked this PR ready for review now.

However, certain applications (like vim) will still not work because they do not turn off quick edit mode.

DHowett · 2021-05-05T02:51:19Z

@msftbot make sure @zadjii-msft and @miniksa sign off

ghost · 2021-05-05T02:51:23Z

Hello @DHowett!

Because you've given me some instructions on how to help merge this pull request, I'll be modifying my merge approach. Here's how I understand your requirements for merging this pull request:

I'll only merge this pull request if it's approved by @zadjii-msft and @miniksa

If this doesn't seem right to you, you can tell me to cancel these instructions and use the auto-merge policy that has been configured for this repository. Try telling me "forget everything I just told you".

DHowett · 2021-05-05T02:52:16Z

I want @zadjii-msft to sign off because he probably has an opinion about InputBuffer having hardcoded VT sequences in it, and I want @miniksa to sign off because he probably has an opinion about whether this is the correct thing to do in general. Both Mikes are equipped to have that second opinion, but since this is closer to the conhost side... you know. 😄

src/host/inputBuffer.cpp

zadjii-msft · 2021-05-05T21:01:08Z

certain applications (like vim) will still not work because they do not turn off quick edit mode.

wait then how the heck does mouse ever work in vim.exe? Does it require users to manually turn quick edit off?

DHowett · 2021-05-05T21:02:49Z

Does it require users to manually turn quick edit off?

IT SURE DOES

Check out the dates on this mailing list post (view in a private browsing tab so Google's panopticon can't force you to log in.)

zadjii-msft · 2021-05-05T21:11:50Z

Wow, what a history lesson. Well, I suppose that's on the vim maintainers then?

or is the hot take that conpty should have QE disabled by default (since QE _doesn't really make sense in conpty)?

zadjii-msft

Yea that's a nit. Just don't want the notes to get lost if we ever move the repo again and break the git blame in 20 years.

src/host/getset.cpp

zadjii-msft · 2021-05-06T12:32:14Z

hot take 2: should we make a PR to vim.exe to have it disable QE mode?

cold take 3: We should add a troubleshooting paragraph to the docs page that explains

why mouse mode might not work in some win32 exes,
how the maintainers of said software can fix that
A list of known applications that won't work right. Like, link to the vim.exe issue tracker for the request to add support (if such an feature tracker exists)

miniksa

Yeah OK. It's still mildly confusing to understand why this works just from the code itself... but the link back to this pull I guess is as good as we can do here. We've definitely done stuff like this before, so I don't have any reason to block it if it helps compatibility broadly.

ghost · 2021-05-07T02:46:08Z

Hello @PankajBhojwani!

Because this pull request has the AutoMerge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (`@msftbot`) and give me an instruction to get started! Learn more here.

omern1 · 2021-05-07T13:31:55Z

Hi, do you have development builds where I can try this out?

ghost · 2021-05-25T19:11:09Z

🎉Windows Terminal Preview v1.9.1445.0 has been released which incorporates this pull request.:tada:

Handy links:

wez · 2021-05-28T16:28:42Z

re: vim.exe quickedit mode; I understand that vim doesn't turn it off, but!
Why does it default to on in terminal preview (and any app that runs openconsole.exe via conpty.dll) when the registry is set to disable it?
Is there a recommended way for a terminal emulator to turn it off by default?

DHowett · 2021-05-28T16:34:03Z

All fair questions.

ConPTY ignores any registry settings, even for the console defaults, because it's intended to provide a console interface that acts the same on all machines regardless of user configuration (so that the hosting terminal can make the final call about a number of things).

It then follows that it defaults to on because that's what it does in an unconfigured conhost 😄

The problem now, though, is this:

ENABLE_MOUSE_INPUT is on by default (because that's also how it's always been), even for things like PowerShell and CMD (!) which do not use mouse events.
Quick edit suppresses mouse mode.
The only way to get mouse events in a Win32 console application is to have quick edit off and mouse mode on.
If we switch ENABLE_QUICK_EDIT_MODE off by default, or allow it as an option, the first sequence you'll get will probably be \e[?1003;1006h. You'll be in mouse mode all the time.
- Won't this make it difficult for a terminal hosting a console application to determine when it should handle mouse events locally (scroll the buffer, select text, open a context menu)? Legitimate question -- I'd be worried about terminal emulators opting into a default experience that makes them more like conhost. The whole point of ConPTY was to let people be less like conhost, and not have to deal with conhost's baggage. 😄

The Windows conpty layer now allows terminal emulators other than the classic conhost.exe to host win32 console applications that use mouse events. For mouse events to function correctly, mouse input needs to be enabled and quick edit mode needs to be disabled. This commit teaches vim to make this change when it enables mouse mode. I have no special knowledge of vim, and this patch is roughly based on the suggested changes in this discussion: https://groups.google.com/g/vim_dev/c/bQ7jfMwa8Zg The intent is to enable mouse input and disable quickedit when enabling mouse mode, and to disable mouse input and restore the prior quickedit mode state when mouse mode is disabled. refs: microsoft/terminal#9970

wez · 2021-05-28T17:13:59Z

Makes sense! tricky!
In the meantime, I went ahead and submitted a PR to vim to teach it to disable quickedit mode

The Windows conpty layer now allows terminal emulators other than the classic conhost.exe to host win32 console applications that use mouse events. For mouse events to function correctly, mouse input needs to be enabled and quick edit mode needs to be disabled. This commit teaches vim to make this change when it enables mouse mode. I have no special knowledge of vim, and this patch is roughly based on the suggested changes in this discussion: https://groups.google.com/g/vim_dev/c/bQ7jfMwa8Zg The intent is to enable mouse input and disable quickedit when enabling mouse mode, and to disable mouse input and restore the prior quickedit mode state when mouse mode is disabled. refs: microsoft/terminal#9970

ghost · 2021-07-14T17:03:47Z

🎉Windows Terminal v1.9.1942.0 has been released which incorporates this pull request.:tada:

Handy links:

Neptune650 · 2021-07-19T21:38:11Z

Makes sense! tricky!
In the meantime, I went ahead and submitted a PR to vim to teach it to disable quickedit mode

And it got merged!, now Win32 Vim supports mouse input.

mouse mode

4fffd4f

PankajBhojwani commented Apr 28, 2021

View reviewed changes

src/host/inputBuffer.hpp Outdated Show resolved Hide resolved

skyline75489 reviewed Apr 28, 2021

View reviewed changes

zadjii-msft reviewed Apr 28, 2021

View reviewed changes