feat: add support for custom keybindings and editor actions in the REPL
Towards #2647.
Description
What is the purpose of this pull request?
This pull request:
- [x] adds editor actions
- [x] adds support for custom keybindings
- [ ] make existing actions configurable
- [ ] adds settings, APIs and update docs
Related Issues
Does this pull request have any related issues?
This pull request:
- partially resolves #2647
Questions
Any questions for reviewers of this pull request?
For the sake of user experience, I decided the key object can have 4 properties: name, ctrl, shift, and meta. In actuality, a readline key object also has properties like sequence and code, which mainly contain the escape sequence of the keypress.
In most keypresses, the name, ctrl, shift, and meta properties are correctly emitted and are enough to determine the action we have to trigger. But in some cases, for instance, CTRL+/, instead of emitting an event with { 'name': '/', 'ctrl': true }, it emits a sequence { 'sequence': '\x1f' }. So if a user has keybindings with CTRL+/ as the key, it won't work, as such a keypress is never emitted.
Now such cases might be limited, as readline only uses the sequence prop to identify two keypress actions, namely undo and redo.
Maybe we can handle these two special cases manually in code? Not sure if a robust solution exists.
Ofcourse, we can also punt the entire responsibility of giving the readline-ready key object, onto the user, but that's not user friendly IMO.
Other
Any other information relevant to this pull request? This may include screenshots, references, and/or implementation notes.
No.
Checklist
Please ensure the following tasks are completed before submitting this pull request.
- [x] Read, understood, and followed the contributing guidelines.
@stdlib-js/reviewers
Another solution to the problem in OP is, we can expect the user to give us the ASCII sequence of the keypress instead.
Every keypress will always have a sequence prop unlike name, ctrl, shift and meta.
Now this is not user-friendly as the user is expected to know the control sequence of the keypress they are wanting to assign.
So, another approach would be to maintain a table mapping all possible keybindings to their corresponding emitted control sequences. Something like:
const CONTROL_SEQUENCES = {
'\x01': 'Ctrl+A',
'\x02': 'Ctrl+B',
'\x03': 'Ctrl+C',
'\x04': 'Ctrl+D',
'\x05': 'Ctrl+E',
'\x06': 'Ctrl+F',
'\x07': 'Ctrl+G',
'\x08': 'Ctrl+H',
'\x09': 'Ctrl+I',
'\x0A': 'Ctrl+J',
'\x0B': 'Ctrl+K',
'\x0C': 'Ctrl+L',
'\x0D': 'Ctrl+M',
'\x0E': 'Ctrl+N',
'\x0F': 'Ctrl+O',
'\x10': 'Ctrl+P',
'\x11': 'Ctrl+Q',
'\x12': 'Ctrl+R',
'\x13': 'Ctrl+S',
'\x14': 'Ctrl+T',
'\x15': 'Ctrl+U',
'\x16': 'Ctrl+V',
'\x17': 'Ctrl+W',
'\x18': 'Ctrl+X',
'\x19': 'Ctrl+Y',
'\x1A': 'Ctrl+Z'
};
But not sure if control sequences and their corresponding keys are standardized across emulators..? How should we proceed?
https://github.com/nodejs/node/blob/main/lib/internal/readline/utils.js#L59
readline parses some keys based on xterm and gnome emulator behaviors (I guess). Maybe we can enforce xterm standards to parse keys? I suppose most combinations might emit standardized sequences..(?)
@Snehil-Shah Not requiring providing a sequence was the right call, I think, given that asking users to know or look up the control sequences for keybindings they want is undesirable. The current approach of a key object with Boolean modifier properties seems good; we could consider renaming meta to alt, as that the latter be understood by more people maybe?
Enforcing xterm standards for key parsing could be feasible. xterm is widely used and AFAIK many terminal emulators aim for compatibility with xterm, so this sounds worth investigating.
I also do think that it would be a pragmatic choice to handle the undo and redo special cases separately.
Now if we decide to go ahead and enforce xterm control sequences, we also have the flexibility to change the input format the user can give. So we can have something like the string "CTRL-E" as input, and map this to the corresponding xterm control sequence..
Went ahead and added parsing of unrecognized keypresses. Fortunately, there were not many cases to cover, the above problems were mainly around keys involving symbols.
@Planeshifter I think it would be better to stick to meta as the internal readline key object, also uses it. This makes it easier to map keybindings. And julia also uses meta instead of alt in their naming so we're not alone.
@Planeshifter Makes sense..
we could use the base package equivalents
We should go ahead and make those changes. Too easy to forget to do them later.
Maybe we can handle these two special cases manually in code? Not sure if a robust solution exists
@Snehil-Shah I'll defer to your judgement on that.
@Snehil-Shah Gentle ping regarding resolving the above comments.
Coverage Report
| Package | Statements | Branches | Functions | Lines |
|---|---|---|---|---|
| repl | $\color{red}11592/14373$ $\color{green}+80.65\%$ |
$\color{red}663/895$ $\color{green}+74.08\%$ |
$\color{red}153/266$ $\color{green}+57.52\%$ |
$\color{red}11592/14373$ $\color{green}+80.65\%$ |
The above coverage report was generated for the changes in this PR.
@kgryte cleaned up some stuff. Regarding keybindings overlapping with system preferences, I don't see a way out.
I don't see a way out
@Snehil-Shah Maybe the way out is just clearly documenting that overlap may happen and that there is likely also going to be some platform variability, so MMV.
@kgryte should I update the docs in this PR itself? I'll raise another PR to add keybindings to existing actions..
@Snehil-Shah Let's punt updating the docs, and let the keybindings fly under the radar for the time being while we test out, and then we can add to docs in a follow-up PR.
PR Commit Message
feat: add support for custom keybindings and editor actions in the REPL
PR-URL: https://github.com/stdlib-js/stdlib/pull/2739
Closes: https://github.com/stdlib-js/stdlib/issues/2647
Co-authored-by: Athan Reines <[email protected]>
Reviewed-by: Philipp Burckhardt <[email protected]>
Signed-off-by: Athan <[email protected]>
Signed-off-by: Snehil Shah <[email protected]>
Please review the above commit message and make any necessary adjustments.