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

Filter out any NUL keystrokes that aren't Ctrl+2/Ctrl+space #3280

Closed
wants to merge 1 commit into from

Conversation

davep
Copy link
Contributor

@davep davep commented Sep 11, 2023

See #872.

@davep davep added the bug Something isn't working label Sep 11, 2023
@davep davep self-assigned this Sep 11, 2023
@davep davep linked an issue Sep 11, 2023 that may be closed by this pull request
@davep
Copy link
Contributor Author

davep commented Sep 11, 2023

Note that this also solves the problem of the Command Palette popping up when you paste something into an input field in Windows.

@davep davep marked this pull request as ready for review September 11, 2023 19:13
@davep davep marked this pull request as draft September 12, 2023 07:18
@davep
Copy link
Contributor Author

davep commented Sep 12, 2023

Taking this back to draft for a moment. While it seems to work fine in Windows under Parallels on my Mac, it's not working fine on the native Windows installation. This might be down to Windows Terminal version or something, amongst other things, so I want to double-check all of that.

@davep
Copy link
Contributor Author

davep commented Sep 12, 2023

Key differences between working and non-working environments:

Working

  • Windows 11
  • Under Parallels
  • Under macOS
  • Windows Terminal 1.17.11461.0

Non-working

  • Windows 10 Pro
  • Native
  • Windows Terminal 1.16.10261.0

Conclusion

The first thing to try is to update the version of Windows Terminal so it's the same in both places.

@davep
Copy link
Contributor Author

davep commented Sep 12, 2023

Updated to the same version of Windows Terminal, on the native Windows 10 machine, and it made not a jot of difference. I'll throw in some low-level debugging again to see what's coming through.

The machine also has the option to be upgraded to Windows 11, but I'm not going to jump into doing that just yet -- having a Windows 10 machine to hand to test with does make a lot of sense; especially if that is the difference.

@davep
Copy link
Contributor Author

davep commented Sep 12, 2023

microsoft/terminal#2865 may be important here.

@davep
Copy link
Contributor Author

davep commented Sep 12, 2023

Even odder. This PR, under Windows 11, under Parallels, on macOS, still supports Ctrl+Space. OTOH @rodrigogiraoserrao reports that Windows 11 native, with this PR, shows nothing for the same key combination; while at the same time without the PR shows ctrl+@.

@TomJGooding
Copy link
Contributor

I've also been doing some testing on my Windows 10 if it helps.

Running textual keys on main and then pressing ctrl on its own shows 'ctrl+@'. Pressing ctrl+space results in 'ctrl+@ showing twice.

With this PR, textual keys shows nothing both after pressing ctrl and ctrl+space.

@davep
Copy link
Contributor Author

davep commented Sep 12, 2023

@TomJGooding Thanks, looks like you're seeing what I'm seeing.

@rodrigogiraoserrao
Copy link
Contributor

rodrigogiraoserrao commented Sep 12, 2023

@TomJGooding Thanks, looks like you're seeing what I'm seeing.

Which, just to make sure we're on the same page, is the same as I was seeing.

@davep
Copy link
Contributor Author

davep commented Sep 12, 2023

So, having tested native on Windows 11 here too now, and seeing the same effect, this little hack doesn't quite solve the problem at all. Or, more to the point: it does seem to solve the problem but at the expense of Ctrl+Space; this might be an acceptable loss if #872 is a bigger problem.

I'm going to leave this as draft while we think on this and explore other options1. Meanwhile we'll move the Command Palette onto a different keyboard combination that we can be more confident about.

Footnotes

  1. AKA, for when I can handle subjecting myself to opening a Windows session again and having to try and do Python coding in the most hostile of environments.

@TomJGooding
Copy link
Contributor

AKA, for when I can handle subjecting myself to opening a Windows session again and having to try and do Python coding in the most hostile of environments.

I guess even if you had Emacs you couldn't set a mark either.... 😉

@davep
Copy link
Contributor Author

davep commented Sep 12, 2023

I guess even if you had Emacs you couldn't set a mark either.... 😉

M-x set-mark-command RET :-P

@TomJGooding
Copy link
Contributor

You may have found this already, but I stumbled on the debug tap mode for Windows Terminal which might be useful?

@davep
Copy link
Contributor Author

davep commented Sep 13, 2023

Thanks, no, didn't know of that (really not a Windows user myself). Note that it's the data coming out of ReadConsoleInputW that's being dealt with here.

@davep
Copy link
Contributor Author

davep commented Sep 28, 2023

Closing this PR as it seems the downsides outweigh how trivial it is, and hopefully there's a more fundamental solution to be had with something like #3411.

@davep davep closed this Sep 28, 2023
@davep davep deleted the windows-ctrl-space-filter branch September 28, 2023 12:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Look at Windows key handling
3 participants