refactor!: graceful handle ReedlineEvent::CtrlC#1051
Conversation
Signed-off-by: tison <wander4096@gmail.com>
|
cc @cptpiepmatz @fdncred in case if you have time to drop a review |
|
@fdncred I'm waiting for this PR to be released to use in my project. Are there any other blockers, or is there some estimation I can expect? |
|
I've been waiting for our patch release which happened just a few minutes ago. I'll probably land this PR tomorrow or the next day. |
|
Thanks for your information! |
|
Thanks! |
| self.editor.reset_undo_stack(); | ||
| Ok(EventStatus::Exits(Signal::CtrlC)) | ||
| if self.editor.is_empty() { | ||
| self.editor.reset_undo_stack(); |
There was a problem hiding this comment.
It is a bit late for the review. But I believe CtrlC should always clear undo stack
There was a problem hiding this comment.
@tisonkun if you want to submit another PR to make that change, we can land it pretty quickly.
|
So, I have been running this for a few day now. Not sure I like this change. I would prefer to have a way to get original functionality back somehow. I want to know what others think about this. CtrlC clears the prompt instead of making a new prompt (that was the point of this PR). But I find myself wanting to copy stuff from my previous prompt which I CtrlC-ed to the current prompt. But its no longer possible since the prompt is gone. Not sure how many people might have similar experience as my but I assume that would be very small number of people. Because of the point mentioned above, CtrlC should probably not clear undo stack (already current behavior, but still mentioning this because I stated the opposite in my previous comment.) Previously, CtrlC-ing from vi_normal mode would take me to vi_insert mode in a new prompt. But after this PR, CtrlC-ing the first time only clears the prompt keeping me in my current mode. Only after CtrlC-ing the second time (when the prompt is empty), it takes me to vi_insert mode. Not sure if I like this change either.But I can get used to this. CtrlC sometimes maintain current mode and sometimes doesn't. I think this is a confusing behavior. It should be consistent. I made this comment to bring attention to the matter that it changes very core behavior the users interact with the most. So we should give it more thorough consideration whether we want this change or want to compromise. |
|
I vote for consistency and making things easier for the user, which falls in line with your idea of not clearing and generally making thigs more consistent. |
|
@NotTheDr01ds I'm up for whatever the right fix is. There's also #1058 although we're getting close to a release again without much time to test. |
|
@NotTheDr01ds i saw it. go for it if that's what you want to do. it's not really bothering me the way it is now. |
If Ctrl+C is hardcoded as before this PR in reedline, I think there is no escape way :O |
4 participants
This closes #1050
This aligns the manner of
ReedlineEvent::CtrlD. I don't know whatself.deactivate_menus();does so do my guess to put it in theif self.editor.is_empty()branch.