Fix crash when filtering commits (#3838)

- **PR Description**

First we fix the crash reported in #3812 (see there for reproduction
recipe), and then we add a more general fix for avoiding crashes in
similar situations that we don't know about yet.

I'm not really happy with the brittleness of all this (the
interdependency between rendering and updating search results, that is),
but I haven't found a good way to untangle this yet.

Fixes #3812.

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

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
 }

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),

pkg/gui/presentation/commits.go

@@ -65,7 +65,7 @@ func GetCommitListDisplayStrings(
 		return nil
 	}
 
-	if startIdx > len(commits) {
+	if startIdx >= len(commits) {
 		return nil
 	}
 

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))