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.

Sign up for GitHub

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

Jump to bottom

Unable to create mappings with colon. #2883

Closed
jordwalke opened this issue Dec 26, 2020 · 5 comments · Fixed by #2902
Closed

Unable to create mappings with colon. #2883

jordwalke opened this issue Dec 26, 2020 · 5 comments · Fixed by #2902
Labels
A-input Area: Input management, keyboard layout, IME etc. bug Something isn't working

Comments

@jordwalke
Copy link

I opened the command prompt to create mappings and it would not accept mappings that involved :.
(I map ; to : in normal mode to save myself the shift key).
I think there may be issues with other kinds of mappings, but I can't recall which exact ones failed. I'll reply here with more info if I come across those again.
@bryphe bryphe added A-input Area: Input management, keyboard layout, IME etc. bug Something isn't working labels Dec 29, 2020
@bryphe
Copy link
Member

bryphe commented Dec 29, 2020 •
edited
Loading

Thanks for logging the issue, @jordwalke !

There's a gap in our input handling code right now - it's only handling direct key productions (so our input code sees : as shift+;, which isn't great... since it doesn't handle multiple keyboard layouts). When we try to bind :, we get a failure since the current code can't find a : keycode on the keyboard.

Various keys that require shift pressed that would be expected to bind correctly like +, ?, _ are also impacted by this (and AltGr keys...).

@zbaylin set up the framework in #2687 for us to handle this correctly - I need to get it hooked up.

Also related to #2114 and #2293

This was referenced Dec 30, 2020
Merged
Merged
bryphe added a commit that referenced this issue Jan 1, 2021
@bryphe
fix(input/#2883): Handle alternate input productions (#2902)
a55e692 
Fixes #2883 

__TODO:__
- [x] Make use of our new keyboard layout module (thanks @zbaylin !)
- [x] Bring back special-key handling
- [x] Add test case for #2883 
- [x] Fix scancodes / verify 'AllKeysReleased' behavior for Control+Tab
- [x] Fix bug with C-C / C-D / C-U being forwarded to Vim
- [x] Fix newly introduced bug with Control+Shift+P/Cmd+Shift+P

__Next steps:__
- #2114 - respect 'no recursive' flag for remaps (#2908)
- #2293 - Update the `keyDown` function in `EditorInput` to take a list of candidates as opposed to a single keypress.
- #2293 - Enable some bindings that were previously blocked:
  - `<C-W>+`
  - `<C-W>_`
  - `<C-W>|`
- Refactor input event subscription and `Sld2.TextInput.start` out of store and into a `Service_Input` sub
- Move `input` utility to common integration test infrastructure
- Remove the `KeyboardInput` Action
bryphe added a commit that referenced this issue Jan 1, 2021
@bryphe
Revert "fix(input/#2883): Handle alternate input productions (#2902)"
a5889dd 
This reverts commit a55e692.
@bryphe bryphe reopened this Jan 1, 2021
@bryphe
Copy link
Member

bryphe commented Jan 1, 2021

Some regressions have been identified with #2902, so I'll back it out and work through bringing it back in

bryphe added a commit that referenced this issue Jan 1, 2021
@bryphe
Revert "Revert "fix(input/#2883): Handle alternate input productions (#…
425f2ac 
…2902)""

This reverts commit a5889dd.
@bryphe
Copy link
Member

bryphe commented Jan 1, 2021

Actually - tracking those issues separately in #2916 - fix still in 😄

@bryphe bryphe closed this as completed Jan 1, 2021
bryphe added a commit that referenced this issue Jan 1, 2021
@bryphe
fix(input/#2114): Handle nnoremap correctly (#2908)
8a1739e 
__Issue:__ All of our bindings, whether defined by `nmap` or `nnoremap`, were allowing recursive definitions. This breaks the case in #2114, where the remaps would bounce back and forth until they hit our recursion limit.

__Defect:__ We weren't gating recursive remaps, aside from the recursion limit.

__Fix:__ Define and handle no-recursive remaps correctly.

__Todo:__
- [x] Update existing test cases to pick up new API
- [x] Add test cases to validate non-recursive maps
- [x] Get tests green
- [x] Add integration test covering #2114 
- [x] Update CHANGES
- [x] Rebase on #2883 when merged

Fixes #2114

Depends on #2883 

Related #1551
@jordwalke
Copy link
Author

Looks like the latest release fixes this. Also fixed my other mapping inoremap <C-a> <c-o>^!
Update process was so smooth, thank you!

@bryphe
Copy link
Member

bryphe commented Jan 8, 2021

Awesome - thanks @jordwalke for confirming! (and thanks @zbaylin for all the work setting up auto-update 😄 )

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-input Area: Input management, keyboard layout, IME etc. bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix(input/#2883): Handle alternate input productions onivim/oni2
2 participants
@jordwalke @bryphe