-
Notifications
You must be signed in to change notification settings - Fork 844
Refactor edit commands and create readline for vi mode #1885
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
base: main
Are you sure you want to change the base?
Conversation
|
I haven't noticed any issues with this while dogfooding it for the last two weeks, including both inside and outside of terminal, so it looks like I haven't broken the regular commands either. |
3f86c20 to
369f1ef
Compare
369f1ef to
b753413
Compare
nriley
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From the community backlog session — we are waiting on some testing from vi users, but otherwise things look good except for this apparent bug that seems to have snuck through in the earlier version of this code.
|
Do we know how many vi testers we will have? If the concern is testing that nothing breaks outside of this mode, could we just have a few community maintainers use or merge this fork for themselves for a few weeks? That bug was unfortunate, but I caught it fairly soon after the first two weeks --- I just didn't have time to fix it for a while. Otherwise I have been using this code consistently for several months and even outside of terminal, I haven't noticed any further issues. (At one point I thought I had a broken "clear line left" until I discovered that actually that was just not the correct command) |
One of the folks on the call did indeed say they'd do this. Hope we may be able to merge as soon as next weekend. |
b996718 to
1e00e35
Compare
Required reworking of the edit command system somewhat to make it more flexible and overridable
for more information, see https://pre-commit.ci
Co-authored-by: Andreas Arvidsson <andreas.arvidsson87@gmail.com>
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
4579168 to
828ef07
Compare
for more information, see https://pre-commit.ci
kittyuser and edit actions added by BlueDrinks PR #1885
|
|
I'll post these as comments in the code if required. |
|
extend doesn't make sense in a terminal/bash, since there's no no selection. I think redo doesn't have an action mapped by default if ctrl+r goes to rev search… but I'm not sure that's default, have you customised your shell at all? cut and copy use the vi clipboard rather than system, no way around that. I'm not sure default readline bindings have a way to do it either; And regardless, I think you would prefer to have, cut and copy and paste all operate on the terminal selection rather than read line (i.e so you can copy text out of your terminal to the system clipboard) |
Looks like it is only bound in command mode in bash by default. Undo is not bound in zsh's insert mode at all.
Some shells do indeed support a selection, but I agree it makes sense to use the system clipboard. |
tags/terminal/readline_vi.py
Outdated
| normal_cmd("ua") | ||
|
|
||
| def redo(): | ||
| actions.key("escape") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it make more sense to use normal_cmd(".i") here? . seems to be bound as redo both in readline/bash and zsh.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's not redo in the sense that we mean it, it is repeat. readline doesn't have redo; however I still left this here so that in zsh (or other vi emulation) it does work
vi-redo is not the same as a traditional redo, it's actually a repeat command. So no, there is no way to redo |
"ctrl-r" does zsh redo, which it is what we have currently. "." does zsh repeat |
89a0427 to
7a11498
Compare
for more information, see https://pre-commit.ci
Gotcha. Please add comments on expected behavior at the least since bash defaults to reverse i-search in either vi insert or command mode. |
|
How's the comment I currently have? |
Generally speaking the refactor allows you to override how edit actions are applied, which I uses to make sure that the modifier/selection applies by creating a vim motion instead of trying to select terminal text. The refactors to the edit commands are fairly minimal, it's just converting the functions into user actions and refactoring out actions to access the internal dictionaries.
In draft mode while I dog food it for a week or so to ensure it is ready, But reviews are welcome anyway