jesseduffield / lazygit

simple terminal UI for git commands
MIT License
47.94k stars 1.72k forks source link

Fix wrong highlight in staging panel when entering file with only staged changes #3667

Closed stefanhaller closed 1 week ago

stefanhaller commented 2 weeks ago

Reproduction recipe:

  1. stage all changes in a file by pressing space on it in the files panel
  2. enter the staged changes panel by pressing enter
  3. unstage one of the changes

This makes the unstaged changes panel visible, but keeps the focus in the staged changes panel. However, the highlight in the unstaged changes view becomes visible, as if it were focused.

Fixes #3664

codacy-production[bot] commented 2 weeks ago

Coverage summary from Codacy

See diff coverage on Codacy

Coverage variation Diff coverage
Report missing for cf40a5b077343cf6cf3de50b60fc4b47ce929dc1[^1] :white_check_mark: 100.00%
Coverage variation details | | Coverable lines | Covered lines | Coverage | | ------------- | ------------- | ------------- | ------------- | | Common ancestor commit (cf40a5b077343cf6cf3de50b60fc4b47ce929dc1) | Report Missing | Report Missing | Report Missing | | | Head commit (db0a1586d99393cda79e6022f3b3b8b4138b0e8b) | 52746 | 45695 | 86.63% | **Coverage variation** is the difference between the coverage for the head and common ancestor commits of the pull request branch: ` - `
Diff coverage details | | Coverable lines | Covered lines | Diff coverage | | ------------- | ------------- | ------------- | ------------- | | Pull request (#3667) | 23 | 23 | **100.00%** | **Diff coverage** is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: `/ * 100%`

See your quality gate settings    Change summary preferences


:rocket: Don’t miss a bit, follow what’s new on Codacy.

Codacy stopped sending the deprecated coverage status on June 5th, 2024. Learn more [^1]: Codacy didn't receive coverage data for the commit, or there was an error processing the received data. Check your integration for errors and validate that your coverage setup is correct.

stefanhaller commented 2 weeks ago

This branch has a first commit with a band-aid fix, and a lengthy commit message explaining where the problem comes from.

I then reverted that commit and added what I think is a better one, where we clear the highlight in HandleFocusLost; this avoids the problem in a more general way. However, it's also a change in behavior, because now the selection in the files panel stays visible when you enter the staging panel. The reason is that HandleFocusLost is only called when the context is removed from the stack, but not when another context is pushed on top of it. I actually like it this way, because you can see which file you are staging; but it could also be confusing, because it makes it harder to see which panel has the focus.

To address that concern, we could do what many other UI frameworks do that have a concept of "inactive selections", where a view that is on the context stack but is not topmost still draws its selection, but in gray. This is a bit of work and would require a change to gocui, but I think it might be worth it.

@jesseduffield Curious about your opinion on this.

stefanhaller commented 2 weeks ago

I implemented this "inactive selection" mechanism that I talked about above, and I have to say I like it a lot. It also takes effect when opening a panel, so we don't have two bright blue bars at the same time:

image

My only concern is that we need a hex color as the default for this new theme color, and I'm not sure we can rely on all terminals supporting these. Probably not?

jesseduffield commented 1 week ago

Hmm this is tricky: I agree in that I also like having an inactive selection shown to the user, but the need to use a hex code sucks because it means we're not honouring the user's theme (and it may look bad in a light theme compared to a dark theme). How about just bolding the selection?

stefanhaller commented 1 week ago

Works for me, as long as I can overwrite it to gray via config. Just like this you mean: 7a08f096e9a1?