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

Cannot type curly brackets in engine-q #169

Closed
aslynatilla opened this issue Nov 5, 2021 · 25 comments
Closed

Cannot type curly brackets in engine-q #169

aslynatilla opened this issue Nov 5, 2021 · 25 comments
Labels
A-Events Area: Handling of incoming events (e.g. desired keypress not registered) bug Something isn't working P-medium Medium Priority: Bug affects quality of life, feature would improve experience for all

Comments

@aslynatilla
Copy link
Contributor

After building and running engine-q, I cannot type { or }.
I use an italian keyboard, where opening and closing curly brackets implies pressing: Shift + Alt Gr + Key.

Note that even just typing [ or ] seems to present a problem, and it implies pressing: Alt Gr + Key.
In this case, the square brackets are printed, but they can only be cancelled by pressing Backspace twice; it looks like "two characters were pushed" instead of one.

@elferherrera
Copy link
Contributor

In theory, this line should take care of it.

Do you mind running reedline with the flag -k (cargo run -- -k)?
This flag will show you the events that are being pressed in reedline

@aslynatilla
Copy link
Contributor Author

Building and compiling last commit on reedline's main branch prints:

cargo run -- -k
    Finished dev [unoptimized + debuginfo] target(s) in 0.10s
     Running `target\debug\reedline.exe -k`
Ready to print events (Abort with ESC):
Event::Key(KeyEvent { code: Char('a'), modifiers: NONE })
Event::Key(KeyEvent { code: Char('s'), modifiers: NONE })
Event::Key(KeyEvent { code: Char('['), modifiers: CONTROL | ALT })
Event::Key(KeyEvent { code: Char('{'), modifiers: SHIFT | CONTROL | ALT })
Event::Key(KeyEvent { code: Esc, modifiers: NONE })

Everything seems ok, there!

@elferherrera
Copy link
Contributor

Oohh... you see that your input requires SHIFT CONTROL ALT

can you try adding that case to the line I showed you? And run again reedline

@aslynatilla
Copy link
Contributor Author

Adding the following case allows me to write { or }.

(m, KeyCode::Char(c)) if m == KeyModifiers::CONTROL | KeyModifiers::ALT | KeyModifiers::SHIFT => {
   ReedlineEvent::Edit(vec![EditCommand::InsertChar(c)])
}

However, the "phantom character" problem is still there.
Moreover, since this might be related to this issue... I could see the cursor/caret flickering, going from the start to the end of the line.

@sholderbach
Copy link
Member

Would you mind opening a PR for the change?

For the phantom character problem could you check if the non brace AltGr characters (not []{})also cause this issue? I would like to exclude an interaction with the brace matching logic used to decide if enter moves to a new line or executes.

The cursor flickering is a known issue due to the current (re)paint implementation and shouldnt be related to the input side.

@elferherrera
Copy link
Contributor

That cursor flickering happens to me as well. I don't know why it does it.

@sholderbach
Copy link
Member

Could you try typing a [ or { somewhere in the middle of an existing line? Would the cursor actually jump to the end? my wild guess would be that crossterm somehow runs into an unicode normalization issue and the è as a e with accent grave somehow also triggers the Ctrl+e move to end keybinding.

@aslynatilla
Copy link
Contributor Author

@sholderbach I can open the PR with that change, no problem!
Not sure about the interactions... But I can try as soon as I am at my PC again!

@sholderbach
Copy link
Member

On Linux I wasn't able to reproduce the phantom characters with AltGr [ or { on a German keymap.
Your key event log seems to look right so it doesn't directly point to the various edgecases in crossterm related to various modifier keys.

Examples:
crossterm-rs/crossterm#536

crossterm-rs/crossterm#540

@sholderbach
Copy link
Member

Great @aslynatilla!

@aslynatilla
Copy link
Contributor Author

aslynatilla commented Nov 5, 2021

I also tested typing in the middle of an existing line; the cursor jumps back to its "current" position, not the end of the line.
Another discovery I made is that... "the phantom character" stops from pressing Enter and getting the usual message:

Our buffer: <content of the buffer>

Instead, pressing Enter places a new line. Maybe this can reveal something more about the other problem - which probably is not related to the current issue, though.

@sholderbach
Copy link
Member

There is a brace matching logic that will continue to a new line if the brace is not closed included with the demo binary.

so

> {<Cursor><Enter>

should result in

> {
<Cursor>

but after

> {
}<Cursor><Enter>

the program should exit.

Implementation:

fn validate(&self, line: &str) -> ValidationResult {

@aslynatilla
Copy link
Contributor Author

My bad, then, since it looks like it works, consistently, as intended! (I didn't consider/notice that...!)

@sholderbach
Copy link
Member

Can you narrow down for which characters you observe the phantom character 👻 ? Maybe let's compare the signals with the enhanced -k flag

@aslynatilla
Copy link
Contributor Author

aslynatilla commented Nov 5, 2021

It looks like it is related to typing anything that requires AltGr.
An example are @ and # on my keyboard, which are respectively a combination of AltGr + ò and AltGr + à.
Any other key doesn't produce the problem, according to my observations.

EDIT: Also note that running with -k shows that...

Event::Key(KeyEvent { code: Char('['), modifiers: CONTROL | ALT })
Event::Key(KeyEvent { code: Char('{'), modifiers: SHIFT | CONTROL | ALT }) <--- pressed Backspace twice here
Event::Key(KeyEvent { code: Backspace, modifiers: NONE })

...one of the Backspace inputs is not considered. However, pressing another "normal" key works as intended and you may continue typing.

@elferherrera
Copy link
Contributor

@aslynatilla forgot to mention, could you add the same case for the vim event file?

sholderbach pushed a commit that referenced this issue Nov 6, 2021
Support for Shift-AltGr is required for several European keyboards (e.g. italian) #169 #136 #139 #171

* Curly braces fix

* Curly braces fix for vim events

Co-authored-by: Antonio Natilla <[email protected]>
@sholderbach
Copy link
Member

sholderbach commented Nov 6, 2021

@aslynatilla On which platform/terminal emulator are you testing?
So the Euro symbol and the closing ] are not causing the same phenomenon? Then it might point to the accent being somewhat weird.
On the German keyboard accents are handled by keys that are pressed but don't print at first, you either type the vowel for use as accents or a space to get ^ and so on. So they behave like phantom characters on the German keyboard (and seem to be abstracted away by either the os, the terminal emulator or crossterm??) and only pressing the accent key does not bubble up an event to reedline. But that might be wild speculation. Do vowels afterwards get modified?

@aslynatilla
Copy link
Contributor Author

@sholderbach I forgot about the symbol, to be fair. It causes the same phenomenon for me.
Your observations made me double-check something and I can now specify that:

Typing [ prints it, but you need to use Backspace twice to delete the bracket.
However, if you type another character, this is not true: typing [e for example requires two Backspaces in order to delete it completely; typing [{ requires three Backspaces, though - two for deleting the curly bracket, one for deleting the square bracket.

My gut feeling says it might be something related to AltGr... as if pressing that key with another button set a flag that needs to be reset, but is not. It also seems only related to deletion, since insertion after a character using AltGr works fine.

@sholderbach
Copy link
Member

OK either we deal with an invisible character/unicode scalar on the reedline side or there is a bug on the crossterm side.
You could give the new main cargo run -- -k another go if we have different unicode scalar values.
Other than that I was dabbling with JT on a change to how we pull the events from crossterm to drain all events between refreshs but this should only affect timing of events/display stuff but not how the internal string looks like. When I get that finished we might instrument that again and probably have to open the issue with crossterm

@aslynatilla
Copy link
Contributor Author

Tried a run with cargo run -- -k and... it doesn't look like there are different unicode scalar values.
Specifically:

Char: { code: 0x00007b; Modifier SHIFT | CONTROL | ALT; Flags 0b000111
Char: } code: 0x00007d; Modifier SHIFT | CONTROL | ALT; Flags 0b000111
Char: [ code: 0x00005b; Modifier CONTROL | ALT; Flags 0b000110
Char: ] code: 0x00005d; Modifier CONTROL | ALT; Flags 0b000110

It apparently makes sense. The same considerations from above are still valid. You need to press Backspace twice to delete a bracket, if it is the last printed character; it "goes back to normal" if you typed something after that - you can delete both the following character and the bracket by pressing Backspace twice.

@sholderbach sholderbach added the bug Something isn't working label Nov 7, 2021
@sholderbach sholderbach added A-Events Area: Handling of incoming events (e.g. desired keypress not registered) P-medium Medium Priority: Bug affects quality of life, feature would improve experience for all labels Nov 28, 2021
@sholderbach
Copy link
Member

@aslynatilla I would vote for closing the issue here as your fixed the direct input problem with your PR. Would you mind describing the issue of the invisible character to delete on crossterm's bugtracker with info regarding platform, terminal emulator etc.?

@aslynatilla
Copy link
Contributor Author

I won't probably be able to do it tonight, but... it's definitely something I can do in the next days! Should I quote this issue?

@aslynatilla
Copy link
Contributor Author

While I was documenting the bug on crossterm's bug-tracker, I double-checked something I didn't think about earlier.
I have always tried this using nushell as an integrated terminal in Visual Studio Code.
However, running reedline (with cargo run) on the current main branch in nushell, opened as a Window Terminal window, does not present the same issue.
I am now thinking this might be Visual Studio Code's problem, more than crossterm's.

@sholderbach
Copy link
Member

Can you reproduce the same issue with the mainline nushell using rustyline under VS code. VS code has it's own troubled history with key mapping independent of what might be reasonably intercepted as keybindings (e.g. under linux microsoft/vscode#23991 open for years) as it's turtles all the way down from VS code with its components, Electron and the Chromium engine.

@sholderbach
Copy link
Member

OK after discussing with @aslynatilla the remaining invisible character issue seems to be stemming from VS codes internal terminal and key detection. No need to file with crossterm

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Events Area: Handling of incoming events (e.g. desired keypress not registered) bug Something isn't working P-medium Medium Priority: Bug affects quality of life, feature would improve experience for all
Projects
None yet
Development

No branches or pull requests

3 participants