Add support for composed chars and fix issues with "foreign" keyboard layouts #1006

Merged
merged 20 commits into from Nov 16, 2016

Projects

None yet
@matheuss
Collaborator
matheuss commented Nov 16, 2016 edited

kapture 2016-11-16 at 15 22 03

Summary

This PR introduces an attempt to fix all the following issues:

Major:

  • Users on some "foreign" keyboard layouts (Portuguese and Norwegian, for example) weren't able to type characters like ~, ', " and so on
  • Characters that need a composition event couldn't be typed – á, é, ü and so on

Minor:

  • After the bell, the cursor would go back to a BLOCK#674
  • Selecting pane on blurred window requires two clicks – #861

Other than that, this changes should fix the following:

  • Some people can't paste unless Hyper receives a click – #897
  • Issues with copy-pasting – #484 #479

Prebuilt binaries

Check here for a .dmg, a .exe, a .deb and a .rpm 😄

How

The work I did on this PR was just an improvement on top of @rauchg's method – huge thanks to him for developing it! Also, thanks @nfcampos for your work on #214 🙌

We inject a hyper-caret div that has contenteditable="true" inside hterm's cursor. With that, we're able to show an intermediate state of composition – so you can see (and type) the character you want.
When the char is ready, hterm automatically captures it – that means we only need to worry about showing that intermediate state.

Closes #29; Closes #66; Closes #466; Closes #518; Closes #579;

rauchg and others added some commits Oct 24, 2016
@rauchg @matheuss rauchg ime 6ef13a2
@matheuss matheuss Fix code style 6fb5c71
@matheuss matheuss Add visual feedback for composition events 5f66a2d
@matheuss matheuss Temporarily disable `hterm`'s `onKeyDown` hacks 5258850
@matheuss matheuss Replicate the focus/blur state of our caret on the `hterm` caret 93353fe
@matheuss matheuss Fix: focus our caret when there's a tab change 15bfc4a
@matheuss matheuss `caret_` => `hyperCaret` fdb5058
@matheuss matheuss Reorg: move the caret hacks to the `hterm.js` extensions bf19f4c
@matheuss matheuss Remove `console.log` ab7ddb5
@matheuss matheuss Remove the `Dead key` hack and reenable keyboard commands f3c79c8
@matheuss matheuss Add a (temporary?) fix to re-enable text selection 317f0e1
@matheuss matheuss Check for a selection `onMouseUp` instead `onFocus` 4c70b96
@matheuss matheuss Fix wrong buggy hterm's cursor styling on term focus/blur 590f2c8
@matheuss matheuss Fix the cursor style after the bell rings – closes #674 952effe
@matheuss matheuss Enable `acceptFirstMouse` to focus the correct term – closes #861 7cc8a37
@matheuss matheuss Fix code style
365016c
@matheuss matheuss Fix: clear the `hyper-caret` when a char is inserted via the IME dialog af8e495
@matheuss matheuss Remove useless function f600511
@matheuss matheuss Add coments regarding text selection 165fd6d
@matheuss matheuss Fix code style
6de55ce
@rauchg rauchg merged commit 27a20e0 into master Nov 16, 2016

2 of 4 checks passed

continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
continuous-integration/travis-ci/push The Travis CI build is in progress
Details
continuous-integration/appveyor/branch AppVeyor build succeeded
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
@rauchg rauchg deleted the add/ime branch Nov 16, 2016
@rauchg
Contributor
rauchg commented Nov 16, 2016

👊

@sergiovilar

👍

@nfcampos
Collaborator

great to see this finally resolved! :)

@kauegimenes

Finally i can start using hyper!

@YuriBrunetto

Great work, everybody!
👏 👏 👏 👏 👏 👏

@bdc-stripe

zomg this makes me so happy 😍

@jbergstroem

Is there an upstream bug for this? setTimeout seems pretty inefficient.

@frdmn
frdmn commented Nov 16, 2016

Well done! 👍

@albinekb
Collaborator

@jbergstroem there isn't the hterm code is hosted and maintained over at googles git

But i think it's an easy fix and even possible to do just by some more prototype hacking 😏
I'll have a look later this week.

Great job @matheuss

@matheuss
Collaborator

@jbergstroem there's already a setTimeout in hterm's code 😄 I just created another one to add that call 👌 Other than that, the bell will not play more than once per second (guaranteed by hterm's code), so we're good 👌

Thank you @albinekb 🙌

@jbergstroem

@matheuss thanks for elaborating.

+ oldRingBell.call(this);
+ setTimeout(() => {
+ this.restyleCursor_();
+ }, 200);
@rauchg
rauchg Nov 16, 2016 Contributor

We need to add some sort of if check or clear this timeout somewhere :P

@matheuss
matheuss Nov 16, 2016 Collaborator

@rauchg I'm not sure if it's necessary – it's just an extension of the timeout that hterm creates:

hterm.Terminal.prototype.ringBell = function() {
  this.cursorNode_.style.backgroundColor =
      this.scrollPort_.getForegroundColor();

  var self = this;
  setTimeout(function() {
      self.cursorNode_.style.backgroundColor = self.prefs_.get('cursor-color');
    }, 200);
...
@matheuss
matheuss Nov 16, 2016 Collaborator

What do you think?

@kentcdodds

Hmmm... Was this supposed to resolve the emoji issues? I did a fresh download/install and it appears to still not be working well for me... Am I missing something?

hyper

@kentcdodds kentcdodds referenced this pull request in kentcdodds/ama Dec 6, 2016
Closed

Some questions about how to open source #228

@noamcore
noamcore commented Dec 6, 2016

This pull request has not been released yet.

@danilomiranda

Great man! Thank you very much!

@karlpokus

Is 1.0.0 supposed to have support for å, ä and ö? I'm on 1.0.0.1303 and it does not work.

@karlpokus

@noamcore Wierd. Still don't work. My ö returns (arg: 6). Is my keyboard f**ked?

@jbergstroem

@karlpokus works for me too. Perhaps check your locale env?

@karlpokus

@jbergstroem

~ $ locale
LANG="sv.UTF-8"

OS X 10.11.5

@jbergstroem

@karlpokus shouldn't that be sv_SE.UTF-8?

@jmml97
jmml97 commented Dec 15, 2016

hi! I have the same problem as @karlpokus I cannot type á or ñ on 1.0. It returns (arg: 2). What is wrong? My locale shows LANG="es.UTF-8". Tried changing it to LANG="es_ES.UTF-8" but it didn't work...

@noamcore
$ locale
LANG=
LC_COLLATE="C"
LC_CTYPE="UTF-8"
LC_MESSAGES="C"
LC_MONETARY="C"
LC_NUMERIC="C"
LC_TIME="C"
LC_ALL=

screen shot 2016-12-15 at 1 19 02 pm

Which keyboard layout are you using?

@negatron99

On Windows 10, attempting to type Japanese characters using the IME.

When typing the first character, the IME immediately disables itself (a cross in it's notification icon) and only the next character is outputted to the terminal. The IME then appears to enable itself (Hiragana symbol) but it continues this cycle of being disabled and enabled.

@noamcore

@negatron99 Japanese characters are not working even in macOS.

I tried right now, with no success.

@alayii
alayii commented Dec 17, 2016

Chinese input method also not working in macOS 10.12.2

if using macOS Pinyin input method, when I type a, nothing appeared in terminal, type a again, character a appeared, I cannot type spacebar to choose chinese character, and candidate box always blink.

if using Squirrel input method, every typing will directly send a chinese character to terminal, cannot choose.

and character width is not right.

@karlpokus

@jbergstroem Yes it should.

# hyper
$ locale
LANG="sv.UTF-8"

# terminal
$ locale
LANG="sv_SE.UTF-8"

Are they suppose do differ?
@noamcore I am using Svensk - Pro keyboard layout.

@jbergstroem

@karlpokus you should set it (correctly) as part of your shell init. Terminal probably does that for you.

@karlpokus

@jbergstroem So you're saying this setting is on the user - not a bug in hyper? or a temp solution?

@jbergstroem

@karlpokus setting a proper locale should be done in the shell, not in the terminal. Some terminals does it for you though. Anyway, I don't even know if this fixes it for you :)

@karlpokus

@jbergstroem Didn't know the terminal on macOS wasn't a shell :/

Anyways. sv.UTF-8is not a valid option it seems and maybe hyper should consider changing it?

# in both terminal and hyper
~ $ locale -a | grep sv -i
sv_SE
sv_SE.ISO8859-1
sv_SE.ISO8859-15
sv_SE.UTF-8
@kleinfreund

@karlpokus If the wrong value sv.UTF-8 is causing your issue then this is on you to change, not Hyper.

@karlpokus
karlpokus commented Dec 17, 2016 edited

@kleinfreund cool. No Problemo. FWIW I can't remember changing this value myself so I thought this might be a bug in hyper considering my terminal got it right.

edit: Setting the following in .bash_profile works.

export LC_ALL=sv_SV.UTF-8  
export LANG=sv_SV.UTF-8  
@jmml97
jmml97 commented Dec 18, 2016

Doing what @karlpokus says above resolves the problem.

@Simon-L
Simon-L commented Jan 3, 2017 edited

Hi, I've done my best to help with improving this as I agree it's an absolute maximum priority issue.
All my research was done in Hyper 1.0.1 on Linux (Mint 18 Cinnamon) set to French language and without looking at Hyper's code, at least for now.

Here is what I have:
On Debian based distros, as per Debian wiki, one place to look at would be /etc/defaults/locale:

$ cat /etc/defaults/locale
#  File generated by update-locale
LANG="fr_FR.UTF-8"

More specific tools exist on distros using systemd, ie. localectl:

$ localectl 
   System Locale: LANG=fr_FR.UTF-8
       VC Keymap: n/a
      X11 Layout: fr
       X11 Model: pc105
     X11 Variant: oss_latin9

localectl provides a reliable output inside Hyper, as opposed to locale which is set to a wrong value inside Hyper and a correct one in gnome-terminal and in TTYs (ctrl+alt+F1...+F2 etc.)

I have then looked at Node packages used for locale/intl'ization, though they seem to be using LC and LANG environment variables: https://github.com/sindresorhus/os-locale/blob/master/index.js#L9
It should be noted that Node obviously gets it right outside of Hyper: process.env.LANG has the correct value.

It seems Hyper is getting the locale value from the wrong source. I'm still looking for gnome-terminal's way of doing this. Editing either .bashrc or .bash_profile fixes it but I believe that similarly to the many other applications one is running, Hyper should get it right from the start.

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