Fix some obscure selection highlighting bugs - #5990
Merged
Merged
Conversation
The selected line of a view says nothing about whether a selection is drawn over it, or which of the two ways it is drawn in, and those are what the tests coming up are about. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Nothing said that a context leaving the stack takes its selection with it, which the work coming up is about to make the rule for every view. Two places already depend on it and are held together by hand: switching repos, where the view focused in the repo being left is not the one focused in the repo being entered, and tabbing from the suggestions list back to the prompt, which replaces the top of the stack rather than popping it. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Whether a view draws a selection is about to be derived in one place from the context stack, which needs to ask any context — list or not — whether there is something for a selection to sit on. Name the existing flag after that question, and let a list context answer it from its length. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
A refresh only re-derives the highlight of the view that has the focus, so a list whose contents change while the user is somewhere else keeps the selection it had: none for a list that just got its first item, and one over nothing for a list that just lost its last. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
…n view Toggling whitespace re-focuses the side panel to re-render the diff, which also re-derives that panel's highlight — as though the panel had the focus, which it doesn't. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Toggling whitespace needs the panel beneath to render its diff again, which is what HandleRenderToMain is for; HandleFocus does that and also everything else that belongs to a panel gaining the focus, which this panel already has or, when the focus is in the main view, does not want. Re-selecting its current item is harmless, but re-deriving its highlight as a focused panel's is not: the selection turns bright while the user is somewhere else. Changing the context size and switching diff renderers already ask for a re-render this way. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
A view drew a selection because something told it to, from four places on three different schedules: a context being focused, a context losing focus, a context being activated over another one, and a list being re-rendered. Whether the flags ended up describing the state of the app depended on which of those had run last, and the last one to run was often none of them: a refresh only re-focuses the view that has the focus, so a list whose contents changed underneath an unfocused panel kept whichever highlight it happened to have. Derive both flags instead, in one place, from the two things they mean: a view shows a selection while its context is on the stack and has something to select, and the context the user is in shows an active one where the ones behind it show inactive ones. Nothing else needs to say anything about highlighting, so nothing else can leave a view saying something untrue about where the focus is. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two places nudged the flags because nothing else would: switching repos, where the view focused in the repo being left is not the one focused in the repo being entered, and tabbing from the suggestions list back to the prompt, which replaces the top of the stack rather than popping it, so the suggestions context never hears that it lost the focus. Both are just a context leaving the stack now. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 task
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
1 participant
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.
Derive the selection highlight from the context stack.
A view drew a selection because something told it to, from four places on three different schedules: a context being focused, a context losing focus, a context being activated over another one, and a list being re-rendered. Whether the flags ended up describing the state of the app depended on which of those had run last, and the last one to run was often none of them: a refresh only re-focuses the view that has the focus, so a list whose contents changed underneath an unfocused panel kept whichever highlight it happened to have.
Derive both flags instead, in one place, from the two things they mean: a view shows a selection while its context is on the stack and has something to select, and the context the user is in shows an active one where the ones behind it show inactive ones. Nothing else needs to say anything about highlighting, so nothing else can leave a view saying something untrue about where the focus is.
This fixes edge case bugs such as highlighting the main view with a single, untracked file, and then removing the file outside of lazygit; when coming back, the files panel would still draw an inactive selection. Now it gets hidden automatically by way of the new mechanism.