bd7fabef1f
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.
FileTree Package
This package handles the representation of file trees. There are two ways to render files: one is to render them flat, so something like this:
dir1/file1
dir1/file2
file3
And the other is to render them as a tree
dir1/
file1
file2
file3
Internally we represent each of the above as a tree, but with the flat approach there's just a single root node and every path is a direct child of that root. Viewing in 'tree' mode (as opposed to 'flat' mode) allows for collapsing and expanding directories, and lets you perform actions on directories e.g. staging a whole directory. But it takes up more vertical space and sometimes you just want to have a flat view where you can go flick through your files one by one to see the diff.
This package is not concerned about rendering the tree: only representing its internal state.