Reduce the chance of race condition with list cursor

Before this commit, we had pkg/integration/tests/submodule/add.go
failing with a panic. I'm pretty sure the issue is this: we're now
calling quite a few GetDisabledReason calls on each layout() call,
and if a background thread happens to update a model slice while
we're doing this, we can end up with a selection index that's now
out of bounds because it hasn't been clamped to match the new list
length.

Specifically, here we had the selected index being -1 (the list starts
empty and somehow the value is -1 in this case) and then the list
gets a new submodule so the length is now 1, but the list cursor
doesn't know about this so remains on the old value. Then we confirm
the length is greater than zero and try to get the selected submodule
and get an out of bounds error.

This commit fixes the issue by clamping the selected index whenever
we get the length of the list so that it stays in-sync. This is not
a perfect solution because the length can change at any time, but
it seems to reliably fix the test, and using mutexes didn't seem to
make a difference.

Note that we're swapping the order of IFileTree and IListCursor in
the file tree view model to ensure that the list cursor's Len()
method is called (which performs the clamping).

Also, comment from the PR:
This 'trait' pattern we're using is convenient but can lead to awkward
situations. In this case we have both the list view model and the
(embedded) list cursor with a Len() method. The list cursor Len()
method just calls the list view model Len() method. But I wanted
to make it that the list view model now calls ClampSelection() on the
list cursor whenever it obtains the length. This will cause an
infinite loop because ClampSelection() internally calls Len()
(which calls the list view model's Len() method which in turn
calls ClampSelection() again, etc).

The only reason we were passing the list view model into the list
cursor was to supply the length method, so now we're just doing
that directly, and letting the list view model delegate the Len()
call to the list cursor, which now itself calls ClampSelection.

2 files changed, 15 insertions, 15 deletions

diff --git a/pkg/gui/context/list_view_model.go b/pkg/gui/context/list_view_model.go
index bf8c80e23..1cb2ee648 100644
--- a/pkg/gui/context/list_view_model.go
+++ b/pkg/gui/context/list_view_model.go
@@ -20,15 +20,11 @@ func NewListViewModel[T HasID](getModel func() []T) *ListViewModel[T] {
 		getModel: getModel,
 	}
 
-	self.ListCursor = traits.NewListCursor(self)
+	self.ListCursor = traits.NewListCursor(func() int { return len(getModel()) })
 
 	return self
 }
 
-func (self *ListViewModel[T]) Len() int {
-	return len(self.getModel())
-}
-
 func (self *ListViewModel[T]) GetSelected() T {
 	if self.Len() == 0 {
 		return Zero[T]()
diff --git a/pkg/gui/context/traits/list_cursor.go b/pkg/gui/context/traits/list_cursor.go
index e42f1f5e5..3b4ab1c3d 100644
--- a/pkg/gui/context/traits/list_cursor.go
+++ b/pkg/gui/context/traits/list_cursor.go
@@ -5,10 +5,6 @@ import (
 	"github.com/jesseduffield/lazygit/pkg/utils"
 )
 
-type HasLength interface {
-	Len() int
-}
-
 type RangeSelectMode int
 
 const (
@@ -27,15 +23,17 @@ type ListCursor struct {
 	rangeSelectMode RangeSelectMode
 	// value is ignored when rangeSelectMode is RangeSelectModeNone
 	rangeStartIdx int
-	list          HasLength
+	// Get the length of the list. We use this to clamp the selection so that
+	// the selected index is always valid
+	getLength func() int
 }
 
-func NewListCursor(list HasLength) *ListCursor {
+func NewListCursor(getLength func() int) *ListCursor {
 	return &ListCursor{
 		selectedIdx:     0,
 		rangeStartIdx:   0,
 		rangeSelectMode: RangeSelectModeNone,
-		list:            list,
+		getLength:       getLength,
 	}
 }
 
@@ -81,8 +79,9 @@ func (self *ListCursor) GetSelectionRangeAndMode() (int, int, RangeSelectMode) {
 
 func (self *ListCursor) clampValue(value int) int {
 	clampedValue := -1
-	if self.list.Len() > 0 {
-		clampedValue = utils.Clamp(value, 0, self.list.Len()-1)
+	length := self.getLength()
+	if length > 0 {
+		clampedValue = utils.Clamp(value, 0, length-1)
 	}
 
 	return clampedValue
@@ -114,7 +113,12 @@ func (self *ListCursor) ClampSelection() {
 }
 
 func (self *ListCursor) Len() int {
-	return self.list.Len()
+	// The length of the model slice can change at any time, so the selection may
+	// become out of bounds. To reduce the likelihood of this, we clamp the selection
+	// whenever we obtain the length of the model.
+	self.ClampSelection()
+
+	return self.getLength()
 }
 
 func (self *ListCursor) GetRangeStartIdx() (int, bool) {