-
Notifications
You must be signed in to change notification settings - Fork 152
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
Implemented VI mode change inside and delete inside functionality #844
Conversation
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.
Thanks, such a nice convenience of vim to have!
There are certainly some mismatches we will still have due to the clunky event translation (like not advancing to the text object if before or multiplier and redo things that may need some tweaking but that probably needs a rework outside the scope of one PR)
I was waiting for this feature for long. It made me curious about the future plans for nushell repl parser/highlighter, are we going to use treesitter here in reedline in near future? Since without AST info, it's hard to deal with nasty cases where brackets get nested like |
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.
Looking good to get that going, in principle the i{char}
text object parsing could also move to the motion parsing part. The command logic should preempt to insert mode before it is interpreted as a traditional motion. But the gain from that refactor is not as big yet as we only have d
and c
accepting motions at the moment.
Self::ChangeInside(right) if is_valid_change_inside_right(right) => { | ||
let left = bracket_for(right); | ||
vec![ | ||
ReedlineOption::Edit(EditCommand::CutLeftBefore(left)), | ||
ReedlineOption::Edit(EditCommand::CutRightBefore(*right)), | ||
] | ||
} |
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.
I am thinking you could save yourself some duplication here by doing the conversion to the one bracket side when creating the Change/DeleteInisde(char)
but then there would be also something to watch out for correctness.
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.
I would still need to know if it is a right of left side bracket to get the ordering right.. It felt cleaner to have be a separate match statements. I did collapse the Delete and Change conditions together since they were inherently the same.
Another thing that should be addressed at some point.. Since this is basically supplying two actions, it requires two undo commands to get back to the previous state. |
This implements the change in (ci) and delete inside vi mode functionality:
supports [ , ( , ", ', `, {