diff --git a/pkg/gui/context.go b/pkg/gui/context.go index cd959274c..d83b144f2 100644 --- a/pkg/gui/context.go +++ b/pkg/gui/context.go @@ -3,6 +3,7 @@ package gui import ( "sync" + "github.com/jesseduffield/generics/set" "github.com/jesseduffield/lazygit/pkg/gui/context" "github.com/jesseduffield/lazygit/pkg/gui/types" "github.com/jesseduffield/lazygit/pkg/utils" @@ -180,10 +181,6 @@ func (self *ContextMgr) Activate(c types.Context, opts types.OnFocusOpts) { self.gui.helpers.Window.MoveToTopOfWindow(c) inputViewName := c.GetInputViewName() - oldView := self.gui.c.GocuiGui().CurrentView() - if oldView != nil && oldView.Name() != inputViewName { - oldView.HighlightInactive = true - } if _, err := self.gui.c.GocuiGui().SetCurrentView(inputViewName); err != nil { panic(err) } @@ -199,9 +196,37 @@ func (self *ContextMgr) Activate(c types.Context, opts types.OnFocusOpts) { self.gui.c.GocuiGui().Cursor = v.Editable && v.Mask == "" + self.updateSelectionHighlights() + c.HandleFocus(opts) } +// updateSelectionHighlights re-derives which views draw a selection, and which of +// them draw theirs as the active one: 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 the +// active selection while the ones behind it show inactive ones. +// +// Both of those can change, so this is called wherever they do: from Activate, which +// every change to the stack goes through; after a refresh, since that is when the +// contents of a list change; and from whoever tells a context that its content has +// gained or lost something to select. +func (self *ContextMgr) updateSelectionHighlights() { + self.RLock() + defer self.RUnlock() + + onStack := set.NewFromSlice(lo.Map(self.ContextStack, + func(c types.Context, _ int) types.ContextKey { return c.GetKey() })) + currentKey := self.currentContextWithoutLock().GetKey() + + for _, c := range self.allContexts.Flatten() { + // The global context has no view of its own. + if view := c.GetView(); view != nil { + view.Highlight = onStack.Includes(c.GetKey()) && c.HasSelectableContent() + view.HighlightInactive = c.GetKey() != currentKey + } + } +} + func (self *ContextMgr) Current() types.Context { self.RLock() defer self.RUnlock() diff --git a/pkg/gui/context/base_context.go b/pkg/gui/context/base_context.go index 51d2473c3..b5fbf76ce 100644 --- a/pkg/gui/context/base_context.go +++ b/pkg/gui/context/base_context.go @@ -28,7 +28,7 @@ type BaseContext struct { hasControlledBounds bool needsRerenderOnWidthChange types.NeedsRerenderOnWidthChangeLevel needsRerenderOnHeightChange bool - highlightOnFocus bool + hasSelectableContent bool *ParentContextMgr } @@ -49,7 +49,7 @@ type NewBaseContextOpts struct { Focusable bool Transient bool HasUncontrolledBounds bool // negating for the sake of making false the default - HighlightOnFocus bool + HasSelectableContent bool NeedsRerenderOnWidthChange types.NeedsRerenderOnWidthChangeLevel NeedsRerenderOnHeightChange bool @@ -70,7 +70,7 @@ func NewBaseContext(opts NewBaseContextOpts) *BaseContext { focusable: opts.Focusable, transient: opts.Transient, hasControlledBounds: hasControlledBounds, - highlightOnFocus: opts.HighlightOnFocus, + hasSelectableContent: opts.HasSelectableContent, needsRerenderOnWidthChange: opts.NeedsRerenderOnWidthChange, needsRerenderOnHeightChange: opts.NeedsRerenderOnHeightChange, ParentContextMgr: &ParentContextMgr{}, @@ -118,6 +118,10 @@ func (self *BaseContext) GetKind() types.ContextKind { return self.kind } +func (self *BaseContext) HasSelectableContent() bool { + return self.hasSelectableContent +} + func (self *BaseContext) GetKey() types.ContextKey { return self.key } diff --git a/pkg/gui/context/list_context_trait.go b/pkg/gui/context/list_context_trait.go index 77e071991..2e4ae7267 100644 --- a/pkg/gui/context/list_context_trait.go +++ b/pkg/gui/context/list_context_trait.go @@ -37,6 +37,10 @@ type ListContextTrait struct { func (self *ListContextTrait) IsListContext() {} +func (self *ListContextTrait) HasSelectableContent() bool { + return self.list.Len() > 0 +} + func (self *ListContextTrait) FocusLine(scrollIntoView bool) { self.Context.FocusLine(scrollIntoView) @@ -102,8 +106,6 @@ func formatListFooter(selectedLineIdx int, length int) string { func (self *ListContextTrait) HandleFocus(opts types.OnFocusOpts) { self.FocusLine(!opts.KeepScrollPosition) - self.GetViewTrait().SetHighlight(self.list.Len() > 0) - self.Context.HandleFocus(opts) } diff --git a/pkg/gui/context/main_context.go b/pkg/gui/context/main_context.go index c8b6edade..692c6dd5c 100644 --- a/pkg/gui/context/main_context.go +++ b/pkg/gui/context/main_context.go @@ -21,12 +21,12 @@ func NewMainContext( ctx := &MainContext{ SimpleContext: NewSimpleContext( NewBaseContext(NewBaseContextOpts{ - Kind: types.MAIN_CONTEXT, - View: view, - WindowName: windowName, - Key: key, - Focusable: true, - HighlightOnFocus: false, + Kind: types.MAIN_CONTEXT, + View: view, + WindowName: windowName, + Key: key, + Focusable: true, + HasSelectableContent: false, })), SearchTrait: NewSearchTrait(c), } diff --git a/pkg/gui/context/merge_conflicts_context.go b/pkg/gui/context/merge_conflicts_context.go index 2ab446c06..dd1060288 100644 --- a/pkg/gui/context/merge_conflicts_context.go +++ b/pkg/gui/context/merge_conflicts_context.go @@ -35,12 +35,12 @@ func NewMergeConflictsContext( viewModel: viewModel, Context: NewSimpleContext( NewBaseContext(NewBaseContextOpts{ - Kind: types.MAIN_CONTEXT, - View: c.Views().MergeConflicts, - WindowName: "main", - Key: MERGE_CONFLICTS_CONTEXT_KEY, - Focusable: true, - HighlightOnFocus: true, + Kind: types.MAIN_CONTEXT, + View: c.Views().MergeConflicts, + WindowName: "main", + Key: MERGE_CONFLICTS_CONTEXT_KEY, + Focusable: true, + HasSelectableContent: true, }), ), c: c, diff --git a/pkg/gui/context/patch_explorer_context.go b/pkg/gui/context/patch_explorer_context.go index 334c2e374..434de6e58 100644 --- a/pkg/gui/context/patch_explorer_context.go +++ b/pkg/gui/context/patch_explorer_context.go @@ -47,7 +47,7 @@ func NewPatchExplorerContext( Key: key, Kind: types.MAIN_CONTEXT, Focusable: true, - HighlightOnFocus: true, + HasSelectableContent: true, NeedsRerenderOnWidthChange: types.NEEDS_RERENDER_ON_WIDTH_CHANGE_WHEN_WIDTH_CHANGES, })), SearchTrait: NewSearchTrait(c), diff --git a/pkg/gui/context/simple_context.go b/pkg/gui/context/simple_context.go index 626d5dfcf..2de4199e2 100644 --- a/pkg/gui/context/simple_context.go +++ b/pkg/gui/context/simple_context.go @@ -33,10 +33,6 @@ func NewDisplayContext(key types.ContextKey, view *gocui.View, windowName string } func (self *SimpleContext) HandleFocus(opts types.OnFocusOpts) { - if self.highlightOnFocus { - self.GetViewTrait().SetHighlight(true) - } - for _, fn := range self.onFocusFns { fn(opts) } @@ -47,7 +43,6 @@ func (self *SimpleContext) HandleFocus(opts types.OnFocusOpts) { } func (self *SimpleContext) HandleFocusLost(opts types.OnFocusLostOpts) { - self.GetViewTrait().SetHighlight(false) self.view.SetOriginX(0) for _, fn := range self.onFocusLostFns { fn(opts) diff --git a/pkg/gui/context/view_trait.go b/pkg/gui/context/view_trait.go index 8e12e083f..9fc078e61 100644 --- a/pkg/gui/context/view_trait.go +++ b/pkg/gui/context/view_trait.go @@ -43,11 +43,6 @@ func (self *ViewTrait) SetContent(content string) { self.view.SetContent(content) } -func (self *ViewTrait) SetHighlight(highlight bool) { - self.view.Highlight = highlight - self.view.HighlightInactive = false -} - func (self *ViewTrait) SetFooter(value string) { self.view.Footer = value } diff --git a/pkg/gui/controllers/suggestions_controller.go b/pkg/gui/controllers/suggestions_controller.go index 0553050e5..18ee594b2 100644 --- a/pkg/gui/controllers/suggestions_controller.go +++ b/pkg/gui/controllers/suggestions_controller.go @@ -85,7 +85,6 @@ func (self *SuggestionsController) GetMouseKeybindings(opts types.KeybindingsOpt func (self *SuggestionsController) switchToPrompt() error { self.c.Views().Suggestions.Subtitle = "" - self.c.Views().Suggestions.Highlight = false self.c.Context().Replace(self.c.Contexts().Prompt) return nil } diff --git a/pkg/gui/controllers/toggle_whitespace_action.go b/pkg/gui/controllers/toggle_whitespace_action.go index a1ac0c8da..67bb59d86 100644 --- a/pkg/gui/controllers/toggle_whitespace_action.go +++ b/pkg/gui/controllers/toggle_whitespace_action.go @@ -27,6 +27,6 @@ func (self *ToggleWhitespaceAction) Call() error { self.c.UserConfig().Git.IgnoreWhitespaceInDiffView = !self.c.UserConfig().Git.IgnoreWhitespaceInDiffView - self.c.Context().CurrentSide().HandleFocus(types.OnFocusOpts{}) + self.c.Context().CurrentSide().HandleRenderToMain() return nil } diff --git a/pkg/gui/gui.go b/pkg/gui/gui.go index 4de37fec2..801fe14d3 100644 --- a/pkg/gui/gui.go +++ b/pkg/gui/gui.go @@ -598,13 +598,6 @@ func (gui *Gui) resetState(startArgs appTypes.StartArgs) types.Context { // RefreshHelper.onUIThreadUnlessRepoChanged). gui.repoGeneration.Add(1) - // Un-highlight the current view if there is one. The reason we do this is - // that the repo we are switching to might have a different view focused, - // and would then show an inactive highlight for the previous view. - if oldCurrentView := gui.g.CurrentView(); oldCurrentView != nil { - oldCurrentView.Highlight = false - } - worktreePath := gui.git.RepoPaths.WorktreePath() if state := gui.RepoStateMap[Repo(worktreePath)]; state != nil { diff --git a/pkg/gui/types/context.go b/pkg/gui/types/context.go index 2ef798a34..93b92c70e 100644 --- a/pkg/gui/types/context.go +++ b/pkg/gui/types/context.go @@ -75,6 +75,10 @@ type IBaseContext interface { // determined independently. HasControlledBounds() bool + // true if the context holds something for a selection to sit on. Contexts that + // don't show a selection at all say false, and so do lists with nothing in them. + HasSelectableContent() bool + // the total height of the content that the view is currently showing TotalContentHeight() int @@ -225,7 +229,6 @@ type IViewTrait interface { ScrollDown(value int) PageDelta() int SelectedLineIdx() int - SetHighlight(bool) } type OnFocusOpts struct { diff --git a/pkg/gui/view_helpers.go b/pkg/gui/view_helpers.go index 2151a5692..5f3e4c2ab 100644 --- a/pkg/gui/view_helpers.go +++ b/pkg/gui/view_helpers.go @@ -140,6 +140,10 @@ func (gui *Gui) postRefreshUpdate(c types.Context, opts types.OnFocusOpts) { c.HandleRender() + // The render may have given the context its first item, or taken its last one + // away, which decides whether its view draws a selection at all. + gui.State.ContextMgr.updateSelectionHighlights() + if gui.currentViewName() == c.GetInputViewName() { c.HandleFocus(opts) } else { diff --git a/pkg/integration/components/view_driver.go b/pkg/integration/components/view_driver.go index 102a8562a..dfae58c4b 100644 --- a/pkg/integration/components/view_driver.go +++ b/pkg/integration/components/view_driver.go @@ -360,6 +360,42 @@ func (self *ViewDriver) Content(matcher *TextMatcher) *ViewDriver { return self } +// SelectionIsActive asserts that the view draws its selection as the one the user +// is working in. These three assertions read the highlight flags rather than the +// selected lines, which say nothing about whether the selection is drawn at all. +func (self *ViewDriver) SelectionIsActive() *ViewDriver { + self.t.assertWithRetries(func() (bool, string) { + view := self.getView() + ok := view.Highlight && !view.HighlightInactive + return ok, fmt.Sprintf("%s: expected an active selection to be shown, but it wasn't", self.context) + }) + + return self +} + +// SelectionIsInactive asserts that the view draws its selection dimmed, as a panel +// does while the focus is somewhere else. +func (self *ViewDriver) SelectionIsInactive() *ViewDriver { + self.t.assertWithRetries(func() (bool, string) { + view := self.getView() + ok := view.Highlight && view.HighlightInactive + return ok, fmt.Sprintf("%s: expected an inactive selection to be shown, but it wasn't", self.context) + }) + + return self +} + +// SelectionIsHidden asserts that the view draws no selection at all, e.g. a list +// with nothing in it, where there is nothing to select. +func (self *ViewDriver) SelectionIsHidden() *ViewDriver { + self.t.assertWithRetries(func() (bool, string) { + ok := !self.getView().Highlight + return ok, fmt.Sprintf("%s: expected no selection to be shown, but one was", self.context) + }) + + return self +} + // asserts on the selected line of the view. If you are selecting a range, // you should use the SelectedLines method instead. func (self *ViewDriver) SelectedLine(matcher *TextMatcher) *ViewDriver { diff --git a/pkg/integration/tests/test_list.go b/pkg/integration/tests/test_list.go index 5ac4bdcba..0d25ddba6 100644 --- a/pkg/integration/tests/test_list.go +++ b/pkg/integration/tests/test_list.go @@ -526,8 +526,13 @@ var tests = []*components.IntegrationTest{ ui.ReloadSidePanels, ui.ReorderSidePanels, ui.SubCommitsScrollPositionIsReset, + ui.SuggestionsSelectionFollowsTheFocus, + ui.SwitchRepoMovesTheSelection, ui.SwitchTabFromMenu, ui.SwitchTabWithPanelJumpKeys, + ui.ToggleWhitespaceKeepsUnfocusedSelectionDimmed, + ui.UnfocusedListHidesSelectionWhenEmptied, + ui.UnfocusedListShowsSelectionWhenFilled, undo.UndoCheckoutAndDrop, undo.UndoCommit, undo.UndoDrop, diff --git a/pkg/integration/tests/ui/suggestions_selection_follows_the_focus.go b/pkg/integration/tests/ui/suggestions_selection_follows_the_focus.go new file mode 100644 index 000000000..793c58bf8 --- /dev/null +++ b/pkg/integration/tests/ui/suggestions_selection_follows_the_focus.go @@ -0,0 +1,42 @@ +package ui + +import ( + "github.com/jesseduffield/lazygit/pkg/config" + . "github.com/jesseduffield/lazygit/pkg/integration/components" +) + +var SuggestionsSelectionFollowsTheFocus = NewIntegrationTest(NewIntegrationTestArgs{ + Description: "The suggestions list only shows a selection while it, rather than the prompt, has the focus", + ExtraCmdArgs: []string{}, + Skip: false, + SetupConfig: func(config *config.AppConfig) {}, + SetupRepo: func(shell *Shell) { + shell. + EmptyCommit("one"). + NewBranch("branch-to-checkout") + }, + Run: func(t *TestDriver, keys config.KeybindingConfig) { + t.Views().Branches(). + Focus(). + Press(keys.Branches.CheckoutBranchByName) + + t.ExpectPopup().Prompt(). + Title(Equals("Branch name:")). + Type("branch-to"). + SuggestionTopLines(Contains("branch-to-checkout")) + + t.Views().Suggestions().SelectionIsHidden() + + t.Views().Prompt().Press(keys.Universal.TogglePanel) + t.Views().Suggestions(). + IsFocused(). + SelectionIsActive(). + Press(keys.Universal.TogglePanel) + + t.Views().Prompt().IsFocused() + t.Views().Suggestions().SelectionIsHidden() + + t.Views().Prompt().Press(keys.Universal.Return) + t.Views().Branches().IsFocused() + }, +}) diff --git a/pkg/integration/tests/ui/switch_repo_moves_the_selection.go b/pkg/integration/tests/ui/switch_repo_moves_the_selection.go new file mode 100644 index 000000000..eb7c4b2df --- /dev/null +++ b/pkg/integration/tests/ui/switch_repo_moves_the_selection.go @@ -0,0 +1,48 @@ +package ui + +import ( + "path/filepath" + + "github.com/jesseduffield/lazygit/pkg/config" + . "github.com/jesseduffield/lazygit/pkg/integration/components" +) + +var SwitchRepoMovesTheSelection = NewIntegrationTest(NewIntegrationTestArgs{ + Description: "The selection follows the focus of the repo being switched to, rather than the one being left", + ExtraCmdArgs: []string{}, + Skip: false, + SetupConfig: func(config *config.AppConfig) { + otherRepo, _ := filepath.Abs("../other") + config.GetAppState().RecentRepos = []string{otherRepo} + }, + SetupRepo: func(shell *Shell) { + shell.CloneNonBare("other") + }, + Run: func(t *TestDriver, keys config.KeybindingConfig) { + switchToRepo := func(repo string) { + t.GlobalPress(keys.Universal.OpenRecentRepos) + t.ExpectPopup().Menu().Title(Equals("Recent repositories")). + Lines( + Contains(repo).IsSelected(), + Contains("Cancel"), + ).Confirm() + t.Views().Status().Content(Contains(repo + " → master")) + } + + t.Views().Branches(). + Focus(). + SelectionIsActive() + + // The other repo has its own focus, which is the files panel it starts in + switchToRepo("other") + t.Views().Files().IsFocused() + t.Views().Branches().SelectionIsHidden() + + // And coming back, this repo still has the focus we left it with + switchToRepo("repo") + t.Views().Branches(). + IsFocused(). + SelectionIsActive() + t.Views().Files().SelectionIsHidden() + }, +}) diff --git a/pkg/integration/tests/ui/toggle_whitespace_keeps_unfocused_selection_dimmed.go b/pkg/integration/tests/ui/toggle_whitespace_keeps_unfocused_selection_dimmed.go new file mode 100644 index 000000000..9ffbd2878 --- /dev/null +++ b/pkg/integration/tests/ui/toggle_whitespace_keeps_unfocused_selection_dimmed.go @@ -0,0 +1,31 @@ +package ui + +import ( + "github.com/jesseduffield/lazygit/pkg/config" + . "github.com/jesseduffield/lazygit/pkg/integration/components" +) + +var ToggleWhitespaceKeepsUnfocusedSelectionDimmed = NewIntegrationTest(NewIntegrationTestArgs{ + Description: "Toggling whitespace from the main view leaves the panel beneath it showing a dimmed selection", + ExtraCmdArgs: []string{}, + Skip: false, + SetupConfig: func(config *config.AppConfig) {}, + SetupRepo: func(shell *Shell) { + shell.CreateFileAndAdd("file1", "one\n") + shell.Commit("one") + shell.UpdateFile("file1", " one\n") + }, + Run: func(t *TestDriver, keys config.KeybindingConfig) { + t.Views().Files(). + IsFocused(). + Lines(Contains("file1")). + Press(keys.Universal.FocusMainView) + + t.Views().Main(). + IsFocused(). + Press(keys.Universal.ToggleWhitespaceInDiffView) + + t.Views().Files(). + SelectionIsInactive() + }, +}) diff --git a/pkg/integration/tests/ui/unfocused_list_hides_selection_when_emptied.go b/pkg/integration/tests/ui/unfocused_list_hides_selection_when_emptied.go new file mode 100644 index 000000000..eef409cb2 --- /dev/null +++ b/pkg/integration/tests/ui/unfocused_list_hides_selection_when_emptied.go @@ -0,0 +1,36 @@ +package ui + +import ( + "github.com/jesseduffield/lazygit/pkg/config" + . "github.com/jesseduffield/lazygit/pkg/integration/components" +) + +var UnfocusedListHidesSelectionWhenEmptied = NewIntegrationTest(NewIntegrationTestArgs{ + Description: "A list that loses its last item while the focus is elsewhere stops showing a selection", + ExtraCmdArgs: []string{}, + Skip: false, + SetupConfig: func(config *config.AppConfig) {}, + SetupRepo: func(shell *Shell) { + shell.CreateFileAndAdd("file1", "one\n") + shell.Commit("one") + shell.UpdateFile("file1", "two\n") + }, + Run: func(t *TestDriver, keys config.KeybindingConfig) { + t.Views().Files(). + IsFocused(). + Lines(Contains("file1")). + SelectionIsActive(). + Press(keys.Universal.FocusMainView) + + t.Views().Main().IsFocused() + + t.Views().Files(). + SelectionIsInactive(). + Tap(func() { + t.Shell().RunCommand([]string{"git", "checkout", "--", "file1"}) + t.RefreshInBackground() + }). + IsEmpty(). + SelectionIsHidden() + }, +}) diff --git a/pkg/integration/tests/ui/unfocused_list_shows_selection_when_filled.go b/pkg/integration/tests/ui/unfocused_list_shows_selection_when_filled.go new file mode 100644 index 000000000..78a8f210d --- /dev/null +++ b/pkg/integration/tests/ui/unfocused_list_shows_selection_when_filled.go @@ -0,0 +1,34 @@ +package ui + +import ( + "github.com/jesseduffield/lazygit/pkg/config" + . "github.com/jesseduffield/lazygit/pkg/integration/components" +) + +var UnfocusedListShowsSelectionWhenFilled = NewIntegrationTest(NewIntegrationTestArgs{ + Description: "A list that gets its first item while the focus is elsewhere starts showing a selection", + ExtraCmdArgs: []string{}, + Skip: false, + SetupConfig: func(config *config.AppConfig) {}, + SetupRepo: func(shell *Shell) { + shell.CreateFileAndAdd("file1", "one\n") + shell.Commit("one") + }, + Run: func(t *TestDriver, keys config.KeybindingConfig) { + t.Views().Files(). + IsFocused(). + IsEmpty(). + SelectionIsHidden(). + Press(keys.Universal.FocusMainView) + + t.Views().Main().IsFocused() + + t.Views().Files(). + Tap(func() { + t.Shell().CreateFile("file2", "two\n") + t.RefreshInBackground() + }). + Lines(Contains("file2")). + SelectionIsInactive() + }, +})