Retain index position of allBranchesLogCmd when toggling status panel #4556
+70
−11
Conversation
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
added 3 commits
May 11, 2025 17:33
This made some amount of sense in the previous world. If users were using `statusPanelView: dashboard`, then it would make sense for them to go to their first log when they switch over from the dashboard to the log commands by pressing `a`. This doesn't end up working out super well though for users of `statusPanelView: allBranchesLog`, where upon reaching the view with `1`, then would press `a` and see nothing. Upon returning to the view with a `2` and then `1`, they would then see the new value! Overall, it seems better that the command that you are viewing the output of is still the index of the command stored in lazygit's state. If we want to add more code to make it so that lazygit knows it is _not yet_ showing a branch command, and is instead on the dashboard, and make it so that it doesn't rotate at all in that case, we could do that. But that seems like too much work that no user would ever appreciate. I am half assuming here that users who love allBranchesLogCmds, and use multiple of them, are also running with `statusPanelView: allBranchesLog`, so they will not be negatively impacted by this.
It only needs to be focused in order to search through it. Searching through it is particularly useful when you are using the `allBranchesLogCmd` view and you want to search through your log for something useful.
ChrisMcD1
commented
May 12, 2025
| @@ -266,6 +266,7 @@ func (gui *Gui) resetHelpersAndControllers() { | |||
| } | |||
|
|
|||
| for _, context := range []types.Context{ | |||
| gui.State.Contexts.Status, | |||
There was a problem hiding this comment.
I am in no way positive this is the correct way to register a context as focusable, when it has no particular behaviors that it needs besides search-ability.
I just saw this added in https://github.com/jesseduffield/lazygit/pull/4429/files#diff-451b1ef5904657beb9097bcb881b06951fa00d4339d317f67030f556f456012eR269-R282, and some basic manual testing makes it seem like I now have the 0 keybinding I would want and searching works.
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.
Currently the behaviors of pressing
1to jump to the status panel (withgui.StatusPanelView: allBranchesLogCmd) and pressingaonce in the status panel, are identical. This results in the bug described in #4469, where the simple config:rotates between the two commands as you press
1,2,1,2.This PR splits the behavior of
1andasuch that1just shows the current index without mutating anything, andarotates the index, and then displays is. This latter behavior is not without controversy, as it changes existing behavior. See my long commit body for more details. The only way to "fix" this is to add additional understanding into the command as to what view is currently being rendered in the main view, which seemed to be non-trivial with how this view is implemented. If we wanted to fix this behavior, it might make sense to make more drastic changes and add an actual Tab for this allBranchesLog view.Fixes #4469
go generate ./...)