Skip to content
This repository has been archived by the owner on Dec 15, 2022. It is now read-only.

Improve support for international keyboards #144

Merged
merged 24 commits into from
Sep 15, 2016
Merged

Conversation

nathansobo
Copy link
Contributor

@nathansobo nathansobo commented Sep 14, 2016

Closes #35

Now that the new KeyboardEvent.key and KeyboardEvent.code APIs are available in Chrome, we can finally support international keyboard layouts in core without a building tower of fragile hacks. This isn't as straightforward as I'd hoped, but this PR updates our translation of keyboard events to keystroke strings as follows:

AltGraph handling

Linux

The keystroke's primary character is always based on the .key property of the keyboard event. If we determine that the AltGraph key is depressed via KeyboardEvent.getModifierState, we don't render alt as part of the keystroke. On Windows, we also don't render ctrl as part of the keystroke since ctrl-alt- is commonly used as a substitute for the physical AltGraph key.

On macOS

Like Linux, we base the primary character on the .key property of the keyboard event.

Because Macs don't have a dedicated AltGraph key, we need to be more nuanced about interpreting the user's intent when they use the Mac key. If we always interpreted to mean AltGraph, many alt- bindings would become impossible to type on macOS. Instead, we only consider to mean AltGraph when KeyboardEvent.key is in the basic ASCII character range. If is used and the typed character is non-ASCII, we use a native node module to determine the non--modified variant of the character based on the physical location of the key (KeyboardEvent.code) and the current system keyboard layout.

For example, on a Swiss-German layout, we will resolve ⌥G keystrokes to @ rather than alt-g because @ is an ASCII symbol. However, we won't resolve ⌥G to © on a U.S. layout because we consider the ability to bind alt-g more important than typing copyright symbols. However, if you want to type © with ⌥G, that will work fine as long as there are no existing alt-g bindings. As a text editor used to write code, we considered it worthwhile to treat ASCII as a special case and protect the ability to type basic ASCII from any packages that might accidentally shadow those keystrokes with alt- bindings.

On Windows

We treat ctrl-alt- specially in the same way we treat specially on macOS. If a ctrl-alt- keystroke produces an alternative character in the ASCII character set, we'll treat the keystroke as that character. Otherwise we'll treat it as a ctrl-alt- binding.

Key bindings on non-Latin layouts

If you're using a Greek keyboard on macOS, ⌘Σ does not exist as a binding. If you type that physical key combination, it actually resolves to ⌘S, which is the default binding for saving a document.

In Atom, we've tried to copy this behavior. Basically, bindings can be expressed only with Latin characters. So alt-u can be bound and so can alt-ü, but on non-Latin layouts, keystrokes with modifiers convert the typed character to its equivalent on a U.S. layout.

Apologies if this appears to be a culturally insensitive approach... We're trying to match the expected behavior and it seemed like macOS is a good guide in this respect. Please let us know if we're missing some nuance here.

Nathan Sobo and others added 20 commits September 6, 2016 15:41
If alt- or shift-alt- produces an ASCII character on the current layout,
interpret the keystroke as that character. If ctrl or cmd are also 
pressed, don’t use the alternative character for the binding.

For example, on a Swiss layout:
alt-g is interpreted as @
ctrl-alt-g is still interpreted as ctrl-alt-g
This enables cmd-d on a Greek keyboard rather than cmd-δ, which matches
the behavior os OS X. Non-ASCII characters that still fall in the Latin
character set are not converted, and we only apply the conversion when
a modifier is depressed.
Signed-off-by: Nathan Sobo <nathan@github.com>
Signed-off-by: Nathan Sobo <nathan@github.com>
Signed-off-by: Nathan Sobo <nathan@github.com>
The only time that we can't use KeyboardEvent.key is on
macOS when the option key is held down. In that case,
the .key property will contain an alternative character.
In order to match alt bindings, we need to know the character
that the key would have typed if option were not held down.
We determine this character using the keyboard-layout module.

Signed-off-by: Nathan Sobo <nathan@github.com>
Signed-off-by: Nathan Sobo <nathan@github.com>
Signed-off-by: Max Brunsfeld <maxbrunsfeld@github.com>
@ccoenen
Copy link

ccoenen commented Sep 14, 2016

Is there an easy way to "test drive" this change? Can I just download atom alpha and replace the atom-keymap with this change?

@ccoenen
Copy link

ccoenen commented Sep 14, 2016

My Platform: Windows 10 / 64bit (not that it matters) / German Keyboard Layout

I'll go over all the keys that don't work in 1.10 for me and how it compares to the new version. This is sorted by position on the keybard in english text reading direction.

  • ² and ³ AltGr+2 and AltGr+3 worked before, keeps working
  • {[]} AltGr+7 through AltGr+0 worked before, keeps working. (You may wonder how one even programs on a german keybard, right now. And you would be right.)
  • \ AltGr+ß works fine (did not do anything before) 👍
  • @ AltGr+Q will insert an @ symbol and then Autoflow: Reflow Selection my current piece of code. This seems to be two distinct actions, at least from an "undo buffer" point of view. (before it just reflowed with no added @) 💥 but also 👍
  • AltGr+E worked before, keeps working.
  • ~ AltGr++ works fine (did not do anything before) 👍
  • | AltGr+< worked before, keeps working
  • µ AltGr+M works fine (did not do anything before) 👍

Overall a VAST improvement for me. The only problem remaining is the @ / Reflow thing and that even comes with a quick remedy (undo removing only the reflow).

I've used the editor for only a few minutes, but my "OMG I CAN'T TYPE THAT" reflexes are already relaxing. I used to Ctrl+F \ Ctrl+A Ctrl+C ESC Ctrl+V to get a \ into my code. No more! :D

This feels pretty native to me. I hope this holds true to all these other layouts out there!

@nathansobo
Copy link
Contributor Author

@ccoenen What platform are you on? I want to fix the reflow selection problem.

@nathansobo
Copy link
Contributor Author

Okay, so I've narrowed the reflow issue down to a menu item with ctrl-alt-q as its keyboard shortcut. Menu items are outside of our keymap system because they're handled by the OS, so this will probably need to be addressed by making a pass through all default bindings with menu items and changing their bindings.

Does anyone have a suggestion for a better default binding scheme on Windows? There really seems to be very little real estate.

Nathan Sobo added 2 commits September 15, 2016 13:13
Signed-off-by: Max Brunsfeld <maxbrunsfeld@github.com>
Signed-off-by: Max Brunsfeld <maxbrunsfeld@github.com>
This avoids calling the non-existent getModifierState method on our fake keyboard events when running on Travis.
@ccoenen
Copy link

ccoenen commented Sep 15, 2016

There's another funny side-effect that I didn't test before:

On a german keyboard, there's no single key for /, so The Usual shortcut Ctrl+/ (to comment out a block of selected code) is actually Ctrl+Shift+7 - because / is Shift+7.

This shortcut did not work in atom 1.10, it does work in this version. That's a very welcome surprise!

(meaning this might well have fixed #34 along the way)

@fractalf
Copy link

fractalf commented Oct 8, 2016

Hi, I'm on Linux with a Norwegian keyboard (nb_NO)

Everything works fine when writing code, but the Key Binding Resolver showes wrong keys. Problem is that some shortcuts are not working (ctrl+\ => toggle tree view). Read a lot of threads, but couldn't get to the bottom of the problem

I'm willing to contribute to get this fixed, but not really sure what to do.

Cheers

@Ben3eeE
Copy link
Contributor

Ben3eeE commented Oct 8, 2016

@fractalf This should be fixed starting with Atom 1.12-beta. Contribution at this point would be to wait for beta and see if everything works 🎉

@fractalf
Copy link

@Ben3eeE I installed the 1.12-beta0 today and the issue remains. Did I miss some setting? Scanned the settings, but couldn't find any. Weird thing though, the actual CTRL+\ gave CTRL+] earlier. Now it shows CTRL+=.

@Ben3eeE
Copy link
Contributor

Ben3eeE commented Oct 11, 2016

Thanks for following up 🙇. There should not be any setting for this. Which OS are you on? In addition I can think of these questions.

  1. Can you reproduce this in safe mode atom --safe? There might be some issues in combination with the keyboard-localization package which was previously recommended to resolve these issues.
  2. Do you see these results in the keybinding resolver? Ctrl+..
  3. Did you make sure that your operating system is configured to the correct language settings?

/cc: @thomasjo Norsk layout.

@Ben3eeE
Copy link
Contributor

Ben3eeE commented Oct 11, 2016

I see in your comment ☝️ that you are on Linux, the exact version could still help. In addition to the above questions can you please also provide the following:

  1. The output of atom --version
  2. The output of apm --version

@fractalf
Copy link

fractalf commented Oct 12, 2016

Cheers for the follow-up, issue is not resolved, here's some more data..

> cat /proc/version
Linux version 4.4.0-38-generic (buildd@lgw01-58) (gcc version 5.4.0 20160609 (Ubuntu 5.4.0-6ubuntu1~16.04.2) ) #57-Ubuntu SMP Tue Sep 6p 15:42:33 UTC 2016
> cat /etc/issue
Linux Mint 18 Sarah \n \l
> atom-beta --version
Atom    : 1.12.0-beta0
Electron: 1.3.6
Chrome  : 52.0.2743.82
Node    : 6.3.0
> apm --version
apm  1.12.5
npm  3.10.5
node 4.4.5
python 2.7.12
git 2.10.1

Running > atom-beta --safe didn't help.

OK, this is quite weird. In the Key Binding Resolver this is what I get:

  • pressing ctrl, seeing ctrl
  • pressing \, seeing \
  • pressing ctrl+\, but seeing ctrl-=

My locale settings were a bit of, meaning they were set to Swedish (god help me), so I changed them to Norwegian, but that doesn't seem to have any impact either.

> locale
LANG=en_US.UTF-8
LANGUAGE=
LC_CTYPE="en_US.UTF-8"
LC_NUMERIC=nb_NO.UTF-8
LC_TIME=en_US.UTF-8
LC_COLLATE="en_US.UTF-8"
LC_MONETARY=nb_NO.UTF-8
LC_MESSAGES="en_US.UTF-8"
LC_PAPER=nb_NO.UTF-8
LC_NAME=nb_NO.UTF-8
LC_ADDRESS=nb_NO.UTF-8
LC_TELEPHONE=nb_NO.UTF-8
LC_MEASUREMENT=nb_NO.UTF-8
LC_IDENTIFICATION=nb_NO.UTF-8
LC_ALL=

..awaiting further instructions :)

EDIT 1: Oh, and yes the keyboard layout is Norwegian and all works perfectly fine when writing normally. Seems to be problems when combining with ctrl

EDIT 2: I found a CLI utility named xev. This is what I get when I press

  • ctrl
  • \
  • ctrl+\
KeyPress event, serial 37, synthetic NO, window 0x4200001,
    root 0xd5, subw 0x0, time 3784535, (105,-5), root:(806,361),
    state 0x0, keycode 37 (keysym 0xffe3, Control_L), same_screen YES,
    XLookupString gives 0 bytes: 
    XmbLookupString gives 0 bytes: 
    XFilterEvent returns: False

KeyRelease event, serial 37, synthetic NO, window 0x4200001,
    root 0xd5, subw 0x0, time 3784665, (105,-5), root:(806,361),
    state 0x4, keycode 37 (keysym 0xffe3, Control_L), same_screen YES,
    XLookupString gives 0 bytes: 
    XFilterEvent returns: False

KeyPress event, serial 37, synthetic NO, window 0x4200001,
    root 0xd5, subw 0x0, time 3785850, (105,-5), root:(806,361),
    state 0x0, keycode 21 (keysym 0x5c, backslash), same_screen YES,
    XLookupString gives 1 bytes: (5c) "\"
    XmbLookupString gives 1 bytes: (5c) "\"
    XFilterEvent returns: False

KeyRelease event, serial 37, synthetic NO, window 0x4200001,
    root 0xd5, subw 0x0, time 3785948, (105,-5), root:(806,361),
    state 0x0, keycode 21 (keysym 0x5c, backslash), same_screen YES,
    XLookupString gives 1 bytes: (5c) "\"
    XFilterEvent returns: False

KeyPress event, serial 37, synthetic NO, window 0x4200001,
    root 0xd5, subw 0x0, time 3786975, (105,-5), root:(806,361),
    state 0x0, keycode 37 (keysym 0xffe3, Control_L), same_screen YES,
    XLookupString gives 0 bytes: 
    XmbLookupString gives 0 bytes: 
    XFilterEvent returns: False

KeyPress event, serial 37, synthetic NO, window 0x4200001,
    root 0xd5, subw 0x0, time 3788370, (105,-5), root:(806,361),
    state 0x4, keycode 21 (keysym 0x5c, backslash), same_screen YES,
    XLookupString gives 1 bytes: (1c) "�"
    XmbLookupString gives 1 bytes: (1c) "�"
    XFilterEvent returns: False

KeyRelease event, serial 37, synthetic NO, window 0x4200001,
    root 0xd5, subw 0x0, time 3788486, (105,-5), root:(806,361),
    state 0x4, keycode 21 (keysym 0x5c, backslash), same_screen YES,
    XLookupString gives 1 bytes: (1c) "�"
    XFilterEvent returns: False

KeyRelease event, serial 37, synthetic NO, window 0x4200001,
    root 0xd5, subw 0x0, time 3790236, (105,-5), root:(806,361),
    state 0x4, keycode 37 (keysym 0xffe3, Control_L), same_screen YES,
    XLookupString gives 0 bytes: 
    XFilterEvent returns: False

@Ben3eeE
Copy link
Contributor

Ben3eeE commented Oct 12, 2016

/cc: @nathansobo

@nathansobo
Copy link
Contributor Author

nathansobo commented Oct 12, 2016

It looks like Chrome is misreporting the value of KeyboardEvent.key for keystrokes involving modifier keys on Linux. We're going to need to access native APIs to build up a keymap to work around this issue, which will take a few more days. We won't push 1.12 to the stable channel until this is working correctly. Thanks for reporting!

@Stasimon
Copy link

@Ben3eeE #247

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

Successfully merging this pull request may close these issues.

Right ALT not supported (bad for International Keyboards)
6 participants