Repository navigation
fix(engine): report Esc as inapplicable when there is nothing to cancel - #1285
Merged
Merged
Conversation
Contributor
|
Isn't |
Collaborator
|
I think that example is only reprensentational, and not really functional. I will take a look soon, I might want it at a different place or at least extracted into a function. I will be comming back to you with a more detailed review soon. |
Collaborator
|
After some consideration it fine as is. I have now also built and tested it. Seems fine for me to land. |
Collaborator
|
My mistake, landed on the wrong PR. I am fine to land it. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #990.
In regular editing,
ReedlineEvent::Escalways returnedEventStatus::Handled, even when there was nothing to cancel. So a binding likenever got to the host command. That meant users couldn't keep Esc closing the completion menu and also give it a custom action.
Change
Escstill closes menus and clears the selection, just as before. Now it only returnsHandledif a menu was active or the cursor held a selection. Otherwise it returnsInapplicable. This matches howMenuAccept,MenuPageNext, etc. already report "nothing to do", and it letsUntilFoundmove on to the next event.Behaviour stays the same in these cases:
EscinsideMultiple([..., Repaint]), so the batch still counts as handled and still repaints.Escchanged no state anyway. The only difference is that it no longer triggers a needless repaint.Tests
Added engine tests for these cases:
Handled.Handled.Inapplicable.UntilFound, Esc falls through to the next event only when there is nothing to cancel.I haven't been able to compile or run the tests in this environment.
Fixes #990
Happy to adjust anything or close it if it isn't a fit.