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

Improve processing of key events in W32 GUI #10155

Closed
wants to merge 2 commits into from

Conversation

LemonBoy
Copy link
Contributor

This changeset adds what's essentially a replacement for
TranslateMessage that takes into account the user keyboard layout and
enables more events to be processed without having to special-case them.

The use of MapVirtualKey allows Vim to resolve some of the "virtual"
keycodes such as VK_OEM_{1..6} and friends according to the user kb
layout.

We call ToUnicode with an intentionally reduced keyboard state to avoid
a pitfall of TranslateMessage: not every combinations of Ctrl+
produce a keycode (eg. Ctrl+] does not) and thus no WM_CHAR would be
emitted for them. Since we only care about the character rather than its
interaction with Ctrl or Alt let's drop those from the state matrix.

The remaining code performs what TranslateMessage would do and pump the
appropriate messages in the main loop.

Two main differences:

  • Dead keys are handled in-place, no WM_DEADCHAR is emitted.
  • Modifiers are handled together with the keys they're associated to
    rather than leaving them in the input queue.

Please test this patch with different keyboard layouts and try mappings including Ctrl and Alt, I've been using this for a couple of days with no problem at all.

Fixes the problem I noticed on my Windows machine and allows me to map <C-]>.

Fixes #9895 again, in a different way.

CC @mattn and @k-takata, your feedback as seasoned Win32 hackers is much appreciated.

This changeset adds what's essentially a replacement for
TranslateMessage that takes into account the user keyboard layout and
enables more events to be processed without having to special-case them.

The use of MapVirtualKey allows Vim to resolve some of the "virtual"
keycodes such as VK_OEM_{1..6} and friends according to the user kb
layout.

We call ToUnicode with an intentionally reduced keyboard state to avoid
a pitfall of TranslateMessage: not every combinations of Ctrl+<key>
produce a keycode (eg. Ctrl+] does not) and thus no WM_CHAR would be
emitted for them. Since we only care about the character rather than its
interaction with Ctrl or Alt let's drop those from the state matrix.

The remaining code performs what TranslateMessage would do and pump the
appropriate messages in the main loop.

Two main differences:
- Dead keys are handled in-place, no WM_DEADCHAR is emitted.
- Modifiers are handled together with the keys they're associated to
  rather than leaving them in the input queue.
@codecov
Copy link

codecov bot commented Apr 11, 2022

Codecov Report

Merging #10155 (6d7f80e) into master (1655619) will decrease coverage by 1.02%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##           master   #10155      +/-   ##
==========================================
- Coverage   81.97%   80.95%   -1.03%     
==========================================
  Files         167      161       -6     
  Lines      187872   185512    -2360     
  Branches    42368    41940     -428     
==========================================
- Hits       154017   150178    -3839     
- Misses      21506    22789    +1283     
- Partials    12349    12545     +196     
Flag Coverage Δ
huge-clang-none 82.41% <ø> (+0.02%) ⬆️
huge-gcc-none ?
huge-gcc-testgui ?
huge-gcc-unittests ?
linux 82.41% <ø> (-1.57%) ⬇️
mingw-x64-HUGE 0.00% <0.00%> (ø)
mingw-x64-HUGE-gui 77.19% <0.00%> (+0.01%) ⬆️
windows 75.99% <0.00%> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
src/gui_w32.c 34.75% <0.00%> (-0.21%) ⬇️
src/libvterm/src/rect.h 0.00% <0.00%> (-96.78%) ⬇️
src/libvterm/src/state.c 38.39% <0.00%> (-51.23%) ⬇️
src/libvterm/src/keyboard.c 42.39% <0.00%> (-45.24%) ⬇️
src/libvterm/include/vterm.h 0.00% <0.00%> (-44.45%) ⬇️
src/libvterm/src/pen.c 47.60% <0.00%> (-36.67%) ⬇️
src/libvterm/src/parser.c 60.08% <0.00%> (-35.82%) ⬇️
src/libvterm/src/encoding.c 39.39% <0.00%> (-34.14%) ⬇️
src/libvterm/src/vterm.c 39.63% <0.00%> (-26.75%) ⬇️
src/gui_gtk_x11.c 52.11% <0.00%> (-4.97%) ⬇️
... and 124 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1655619...6d7f80e. Read the comment docs.

@LemonBoy
Copy link
Contributor Author

No feedback about this PR? This change is much needed to make gVim able to map keys like <C-]> when using non-entirely-standard keyboard layouts like this one.

@mattn
Copy link
Member

mattn commented Apr 20, 2022

This break IME (input method editor). I can not input multi-byte strings with this patch.

@LemonBoy
Copy link
Contributor Author

This break IME (input method editor). I can not input multi-byte strings with this patch.

Thanks for the feedback, I did try the latest version of this patch with the Japanese IME and now it seems to be working again.

@brammool
Copy link
Contributor

I'll wait for @mattn to confirm.

@mattn
Copy link
Member

mattn commented Apr 21, 2022

I'm okay to merge.

@cmon1701
Copy link

cmon1701 commented Dec 30, 2022

This change breaks key presses of ':' and other keys of neo2's 3rd level.
See https://www.neo-layout.org/
3rd level ("Ebene" in German): https://dl.neo-layout.org/grafik/bilder-einzeln/flat/neo-3-tkl.path.svg
':‘ is pressed via Capslock+; (QWERTY) / mod3+d (neo2).
It works till gvim_8.2.4803 and is broken starting from gvim_8.2.4807 on (independent of x86 or x64).

Can you please fix this patch? 8.2.4807: processing key eveints in Win32 GUI is not ideal
Thanks!!

@chrisbra
Copy link
Member

@cmon1701 can you check, if #10823 or #11760 fixes it properly? (I am not actually sure, if #11760 is a sub- or superset of #10823)

@cmon1701
Copy link

cmon1701 commented Dec 30, 2022

Hi @chrisbra,
thanks for your quick response!
I have never compiled vim (Win GUI) so far. Are there any executables that incorporate your mentioned fixes or could you please provide them for me? Thanks!
(This is also broken in the newest version, 9.0.1103.)

@chrisbra
Copy link
Member

unfortunately, there doesn't seem to be a way to download those compiled binaries. I have now created #11762 to address this and give users the possibility to download the artifcats and test that. That means, once it is merged and PR authors rebase their work on top of it, you can then download artifacts and test those changes.

@cmon1701
Copy link

@chrisbra thank you!!

@zewpo
Copy link
Contributor

zewpo commented Dec 31, 2022

@chrisbra and @cmon1701
I believe that #11760 is a superset of #10823
@haron13-2019 [@ant0sha] can confirm.

@cmon1701 when you can access a build of gvim (I'm not sure how @chrisbra #11762 works yet), but if you can get at least
Patch 9.0.1112 (which includes #11760), then you should be able to check if it resolves the issue regarding "keys of neo2's 3rd level"

@chrisbra
Copy link
Member

chrisbra commented Jan 1, 2023

You go to the Appveyor CI page for vim and click on the PR that you are interested in. E.g. for #11767 you click on https://ci.appveyor.com/project/chrisbra/vim/builds/45810751 and there you will see the last Appveyor (windows builds and tests) results for that particular build. You will have a tab called artifacts, from which you can then download those build binaries (vim.exe and gvim.exe)

@zewpo
Copy link
Contributor

zewpo commented Jan 1, 2023

Cool, thanks @chrisbra

@sme-prus001
Copy link

Ah, I've left my comment on the code page. After 4807 you can't press Ctrl+S (Windows 10) in Cyrillic layout in insert mode - you get 'ы' instead. You can't underestimate the lack of fast saving. The same is for other Ctrl keys too, like Ctrl+A, Ctrl+C, Ctrl+V etc. I didn't try 4806 but in 4803 everything was ok.

@zewpo
Copy link
Contributor

zewpo commented Jan 20, 2023

Hi @sme-prus001

This PR was done over 10 months ago, and since then more recent changes have addressed some Ctrl+... functionality, at least I'm aware of the Ctrl+_ behavior was put back.

Would it be possible for you to confirm if this is still a problem with the latest gVim? If you need to, you can grab an installer from here: https://github.com/vim/vim-win32-installer/releases

I'll see if I can add some test scripts to cover this.

@sme-prus001
Copy link

sme-prus001 commented Jan 21, 2023 via email

@zewpo
Copy link
Contributor

zewpo commented Jan 21, 2023

@sme-prus001 thanks for the info. I'll try and test it.

@risa2000
Copy link

@LemonBoy
As I just found out (and described in #12595) the changes which possibly started here broke key handling in vim and gVim on Windows with Czech keyboard layout.

It seems that vim and gVim started to mangle the key codes from the keyboard. It is still unclear for me why, but the current state is that pressing Ctrl-<right key to P> on Czech keyboard layout in Windows produced Esc(code) for past 20 years in vim, until recently. Now it produces either plain character (ignores Ctrl) in gVim, or produces wrong Ctrl code in vim (see my issue for details).

Just for the record here, I will reiterate that the keyboard still emits Esc(code) on pressing Ctrl-<right key to P>, because using the same Czech keyboard layout in same Windows, while connected via putty to linux terminal on linux box, and running vim 9 in there works as expected. So the problem is definitely in vim and in particular in vim handling of the key codes on Windows.

Could you confirm that the changes you committed, modify, or filter key codes received from the OS?
If yes, could you explain, what was your goal?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Can't map ctrl slash in nightly gVIM build on Windows
9 participants