From 595c7ee73e9d2053fc1cb4ca0b6fd1274852ee9a Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Thu, 11 May 2023 13:21:24 +0200 Subject: [PATCH 1/2] Extend one of the filtering tests to start on a commit other than the first Enabling the filter selects the first entry in the filtered commits view. It's useful to have a test that checks this, as I almost broke it in the following commit (it needs an added FocusLine call in the setFiltering function in filtering_menu_action.go). --- pkg/integration/tests/filter_by_path/select_file.go | 4 +++- pkg/integration/tests/filter_by_path/shared.go | 2 ++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/pkg/integration/tests/filter_by_path/select_file.go b/pkg/integration/tests/filter_by_path/select_file.go index 6c1243dc8..80a69a2b4 100644 --- a/pkg/integration/tests/filter_by_path/select_file.go +++ b/pkg/integration/tests/filter_by_path/select_file.go @@ -18,10 +18,12 @@ var SelectFile = NewIntegrationTest(NewIntegrationTestArgs{ t.Views().Commits(). Focus(). Lines( - Contains(`only filterFile`).IsSelected(), + Contains(`none of the two`).IsSelected(), + Contains(`only filterFile`), Contains(`only otherFile`), Contains(`both files`), ). + SelectNextItem(). PressEnter() // when you click into the commit itself, you see all files from that commit diff --git a/pkg/integration/tests/filter_by_path/shared.go b/pkg/integration/tests/filter_by_path/shared.go index 93e5e0a3d..675a2d9df 100644 --- a/pkg/integration/tests/filter_by_path/shared.go +++ b/pkg/integration/tests/filter_by_path/shared.go @@ -14,6 +14,8 @@ func commonSetup(shell *Shell) { shell.UpdateFileAndAdd("filterFile", "new filterFile content") shell.Commit("only filterFile") + + shell.EmptyCommit("none of the two") } func postFilterTest(t *TestDriver) { From e5dd4d311064c2b45dfe3f08a2a4cbf4536404cc Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Sat, 6 May 2023 14:02:14 +0200 Subject: [PATCH 2/2] Allow the selected line of a list view to be outside the visible area I don't see a reason why this restriction to have the selection be always visible was necessary. Removing it has two benefits: 1. Scrolling a list view doesn't change the selection. A common scenario: you look at one of the commits of your current branch; you want to see the how many'th commit this is, but the beginning of the branch is scrolled off the bottom of the commits panel. You scroll down to find the beginning of your branch, but this changes the selection and shows a different commit now - not what you want. 2. It is possible to scroll a panel that is not the current one without changing the focus to it. That's how windows in other GUIs usually behave. --- pkg/gui/controllers/filtering_menu_action.go | 1 + pkg/gui/controllers/list_controller.go | 16 ---------------- pkg/gui/controllers/stash_controller.go | 1 + pkg/gui/layout.go | 2 -- 4 files changed, 2 insertions(+), 18 deletions(-) diff --git a/pkg/gui/controllers/filtering_menu_action.go b/pkg/gui/controllers/filtering_menu_action.go index 7c9de7973..f815d5b3f 100644 --- a/pkg/gui/controllers/filtering_menu_action.go +++ b/pkg/gui/controllers/filtering_menu_action.go @@ -74,5 +74,6 @@ func (self *FilteringMenuAction) setFiltering(path string) error { return self.c.Refresh(types.RefreshOptions{Scope: []types.RefreshableView{types.COMMITS}, Then: func() { self.c.Contexts().LocalCommits.SetSelectedLineIdx(0) + self.c.Contexts().LocalCommits.FocusLine() }}) } diff --git a/pkg/gui/controllers/list_controller.go b/pkg/gui/controllers/list_controller.go index 0a6de821a..1ca3a1e12 100644 --- a/pkg/gui/controllers/list_controller.go +++ b/pkg/gui/controllers/list_controller.go @@ -54,12 +54,6 @@ func (self *ListController) HandleScrollUp() error { scrollHeight := self.c.UserConfig.Gui.ScrollHeight self.context.GetViewTrait().ScrollUp(scrollHeight) - // we only need to do a line change if our line has been pushed out of the viewport, because - // at the moment much logic depends on the selected line always being visible - if !self.isSelectedLineInViewPort() { - return self.handleLineChange(-scrollHeight) - } - return nil } @@ -67,19 +61,9 @@ func (self *ListController) HandleScrollDown() error { scrollHeight := self.c.UserConfig.Gui.ScrollHeight self.context.GetViewTrait().ScrollDown(scrollHeight) - if !self.isSelectedLineInViewPort() { - return self.handleLineChange(scrollHeight) - } - return nil } -func (self *ListController) isSelectedLineInViewPort() bool { - selectedLineIdx := self.context.GetList().GetSelectedLineIdx() - startIdx, length := self.context.GetViewTrait().ViewPortYBounds() - return selectedLineIdx >= startIdx && selectedLineIdx < startIdx+length -} - func (self *ListController) scrollHorizontal(scrollFunc func()) error { scrollFunc() diff --git a/pkg/gui/controllers/stash_controller.go b/pkg/gui/controllers/stash_controller.go index 3e1b65ce8..4b5135884 100644 --- a/pkg/gui/controllers/stash_controller.go +++ b/pkg/gui/controllers/stash_controller.go @@ -187,6 +187,7 @@ func (self *StashController) handleRenameStashEntry(stashEntry *models.StashEntr return err } self.context().SetSelectedLineIdx(0) // Select the renamed stash + self.context().FocusLine() return nil }, }) diff --git a/pkg/gui/layout.go b/pkg/gui/layout.go index 5b3e6845f..35dbe55b7 100644 --- a/pkg/gui/layout.go +++ b/pkg/gui/layout.go @@ -131,8 +131,6 @@ func (gui *Gui) layout(g *gocui.Gui) error { continue } - listContext.FocusLine() - view.SelBgColor = theme.GocuiSelectedLineBgColor // I doubt this is expensive though it's admittedly redundant after the first render