History fix cursor update#97
Conversation
we will most likely store multiline commands in history as `String`s so I have opted for a converstion of this string to a vector separated by newlines in `set_buffer`.
- On normal up down we update the cursor to the end of the line - On prefix based up and down we do not update the cursor Rationale for this is that when we are doing prefix search a user might want to add more to his query so as to get better prefix matching and having to delete through all the chars we would otherwise fill in might be annoying.
|
Still some usability painpoints:
For the last two points the iterator like interface of moving while either returning Some(entry) when a move was succesful or None when exhausted had some benefits over the new setup. We want to be able to handle those cases. I started some changes on that, if I get them to work I will file a PR. |
|
OK Update regarding point two on my list. zsh's history traversal would also stop at this point as it then thinks it is in prefix mode. Yet bash allows horizontal cursor movement without interfering with the basic up/down history navigation. So this seems to be somewhat of an UI/UX choice. |
IIRC this has existed for a long time, I wasn't really focused on this one. I really think we should get rid of the clock update for now and introduce it later down the road.
Can you elaborate on this one (ideally with an example). So, I can understand what the behaviour should be. I believe the present state (in the PR) should be that left/right movement etc should stop history searching altogether for Normal and PrefixSearch modes. That is what I wanted to implement.
Yeah, I plan to fix this one.
I had not really thought this through. Definitely, something that should happen. |
|
@sherubthakur - taking a look at this now. I'm seeing something related to a previous PR you did to clean up history and I think it'd be great to fix it as part of this update. If you press up/down and wait a second, the input disappears. Before #92 this worked fine, but after that it seems that after the second ticks by we only draw the prompt and not the user's input. |
sholderbach
left a comment
sholderbach
left a comment
There was a problem hiding this comment.
Let's get this merged that we can remove this history bug, and check for the new interactions.
If the refactored line buffer will have to be taken in to account depends on what gets merged first.
| @@ -65,8 +65,17 @@ impl LineBuffer { | |||
|
|
|||
| /// Set to a single line of `buffer` and reset the `InsertionPoint` cursor | |||
| pub fn set_buffer(&mut self, buffer: String) { | |||
There was a problem hiding this comment.
Are other users of set_buffer maybe expecting a different behavior? Else we may want to have have set_buffer_cursor_end or something else.
| self.history.set_navigation(HistoryNavigationQuery::Normal); | ||
| } | ||
| (false, HistoryNavigationQuery::PrefixSearch(_)) => {} | ||
| (false, HistoryNavigationQuery::PrefixSearch(prefix)) => { |
There was a problem hiding this comment.
Out of curiosity: Against which state is this guarding?
There was a problem hiding this comment.
Each up/down press will call this function. if the prefix does not change we don't want to reset the history state.
|
Yeah I'm also 👍 on landing and continuing to polish. @sherubthakur thanks for the fixes! |
3 participants
Various fixes to history execution