From 926061557bc1dd587b83acec6878253eaab933c3 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Fri, 9 Aug 2024 14:26:29 +0200 Subject: [PATCH 1/5] Make searching available in the filtered commits list It is already possible to search a filtered list by searching first, and then enabling a filter, so I found it inconsistent to not allow searching when you are already filtering. One reason for not allowing this might be that the search status (on the left) hides the filter status (on the right), but if we think that's enough reason to not allow both at the same time, then we should cancel a search when we enter filtering. --- pkg/gui/controllers/local_commits_controller.go | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/pkg/gui/controllers/local_commits_controller.go b/pkg/gui/controllers/local_commits_controller.go index 3900d7f32..2d4232f33 100644 --- a/pkg/gui/controllers/local_commits_controller.go +++ b/pkg/gui/controllers/local_commits_controller.go @@ -207,14 +207,8 @@ func (self *LocalCommitsController) GetKeybindings(opts types.KeybindingsOpts) [ Description: self.c.Tr.MarkAsBaseCommit, Tooltip: self.c.Tr.MarkAsBaseCommitTooltip, }, - // overriding these navigation keybindings because we might need to load + // overriding this navigation keybinding because we might need to load // more commits on demand - { - Key: opts.GetKey(opts.Config.Universal.StartSearch), - Handler: self.openSearch, - Description: self.c.Tr.StartSearch, - Tag: "navigation", - }, { Key: opts.GetKey(opts.Config.Universal.GotoBottom), Handler: self.gotoBottom, @@ -228,6 +222,14 @@ func (self *LocalCommitsController) GetKeybindings(opts types.KeybindingsOpts) [ } bindings := append(outsideFilterModeBindings, []*types.Binding{ + // overriding this navigation keybinding because we might need to load + // more commits on demand + { + Key: opts.GetKey(opts.Config.Universal.StartSearch), + Handler: self.openSearch, + Description: self.c.Tr.StartSearch, + Tag: "navigation", + }, { Key: opts.GetKey(opts.Config.Commits.AmendToCommit), Handler: self.withItem(self.amendTo), From 15d17e16dd1be12192e783c856b3da1e4a4bf831 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Sun, 18 Aug 2024 21:11:21 +0200 Subject: [PATCH 2/5] Call ReApplySearch after layout This fixes a possible crash when exiting filter mode in the commits panel. --- pkg/gui/controllers/helpers/refresh_helper.go | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/pkg/gui/controllers/helpers/refresh_helper.go b/pkg/gui/controllers/helpers/refresh_helper.go index e29879b9d..579bc9d3e 100644 --- a/pkg/gui/controllers/helpers/refresh_helper.go +++ b/pkg/gui/controllers/helpers/refresh_helper.go @@ -765,8 +765,18 @@ func (self *RefreshHelper) refreshView(context types.Context) error { err := self.c.PostRefreshUpdate(context) - // Re-applying the search must be done after re-rendering the view though, - // so that the "x of y" status is shown correctly. - self.searchHelper.ReApplySearch(context) + self.c.AfterLayout(func() error { + // Re-applying the search must be done after re-rendering the view though, + // so that the "x of y" status is shown correctly. + // + // Also, it must be done after layout, because otherwise FocusPoint + // hasn't been called yet (see ListContextTrait.FocusLine), which means + // that the scroll position might be such that the entire visible + // content is outside the viewport. And this would cause problems in + // searchModelCommits. + self.searchHelper.ReApplySearch(context) + return nil + }) + return err } From c3d5798c6c1be666cb8c11a03a7b054bb934bef6 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Fri, 9 Aug 2024 14:30:13 +0200 Subject: [PATCH 3/5] Fix early exit condition I don't know what this condition is supposed to guard against, or whether we really need it (it was added in 06ca71e955, and the commit message of that commit only says "fix bug"). But if we do need it, then it seems that `>=` is more correct than `>`. --- pkg/gui/presentation/commits.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/gui/presentation/commits.go b/pkg/gui/presentation/commits.go index f9bdb4eb7..234a84e5c 100644 --- a/pkg/gui/presentation/commits.go +++ b/pkg/gui/presentation/commits.go @@ -65,7 +65,7 @@ func GetCommitListDisplayStrings( return nil } - if startIdx > len(commits) { + if startIdx >= len(commits) { return nil } From e6750254114ea9c16a33212b98658040b5cc7b25 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Fri, 9 Aug 2024 14:50:24 +0200 Subject: [PATCH 4/5] Return nil columnPositions when not rendering anything ... instead of returning a slice with a single [0] element. This makes it easier to check whether we have columnPositions. --- pkg/utils/formatting.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/pkg/utils/formatting.go b/pkg/utils/formatting.go index b7817346a..b13a2ffa8 100644 --- a/pkg/utils/formatting.go +++ b/pkg/utils/formatting.go @@ -54,6 +54,10 @@ func WithPadding(str string, padding int, alignment Alignment) string { // returns a list of strings that should be joined with "\n", and an array of // the column positions func RenderDisplayStrings(displayStringsArr [][]string, columnAlignments []Alignment) ([]string, []int) { + if len(displayStringsArr) == 0 { + return []string{}, nil + } + displayStringsArr, columnAlignments, removedColumns := excludeBlankColumns(displayStringsArr, columnAlignments) padWidths := getPadWidths(displayStringsArr) columnConfigs := make([]ColumnConfig, len(padWidths)) From af87cd1dd6e58e9472353d783b7c148d925c7a05 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Fri, 9 Aug 2024 14:51:01 +0200 Subject: [PATCH 5/5] Don't return model search results for commits when we don't have columnPositions We fixed one specific scenario where this happened ealier in this branch, but in case there are more that we don't know about yet, at least make sure we don't crash. --- pkg/gui/context/local_commits_context.go | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/pkg/gui/context/local_commits_context.go b/pkg/gui/context/local_commits_context.go index 0cb7a628c..eecb16107 100644 --- a/pkg/gui/context/local_commits_context.go +++ b/pkg/gui/context/local_commits_context.go @@ -202,6 +202,15 @@ func shouldShowGraph(c *ContextCommon) bool { } func searchModelCommits(caseSensitive bool, commits []*models.Commit, columnPositions []int, searchStr string) []gocui.SearchPosition { + if columnPositions == nil { + // This should never happen. We are being called at a time where our + // entire view content is scrolled out of view, so that we didn't draw + // anything the last time we rendered. If we run into a scenario where + // this happens, we should fix it, but until we found them all, at least + // make sure we don't crash. + return []gocui.SearchPosition{} + } + normalize := lo.Ternary(caseSensitive, func(s string) string { return s }, strings.ToLower) return lo.FilterMap(commits, func(commit *models.Commit, idx int) (gocui.SearchPosition, bool) { // The XStart and XEnd values are only used if the search string can't