Fix behaviors of gui.sidePanelWidth: 1

[ChrisMcD1]

• PR Description

We have technically allowed sidePanelWidth = 1 in the schema for forever. But it seems it never worked! #4582

yaml:"sidePanelWidth" jsonschema:"maximum=1,minimum=0"

This PR fixes 2 behaviors:

1. When trying to full screen the main view, we previously we were relying on setting just the sideSectionWeight in order to ensure the main panel was fully focused. This can cause a problem if the user had their gui.sidePanelWidth set to 1. Such a value make the mainPanelWidth = 0 by default. By just setting the sideSectionWeight, you then communicate to the UI that you want both halves of the screen to have 0 width! No good! Crash!
2. When switching to the main view, we implicitly assumed it was visible. Now, we set it to be full screen if it otherwise would have been not-rendered. That feels like the natural behavior for a full-screen side panel user.

These changes seem like they won't impact users of more standard gui.sidePanelWidth values, so it's just a free win!

Fixes #4582

• Please check if the PR fulfills these requirements

• Cheatsheets are up-to-date (run go generate ./...)
• Code has been formatted (see here)
• Tests have been added/updated (see here for the integration test guide)
• Text is internationalised (see here)
• If a new UserConfig entry was added, make sure it can be hot-reloaded (see here)
• Docs have been updated if necessary
• You've read through your own file changes for silly mistakes etc

[stefanhaller]

The changes make sense to me. I guess there are some more weirdnesses around focusing the main view in half or full screen mode, but I don't use these modes much, and can't be bothered right now to explore this more. 😄

We have fancy tests for this kind of stuff in window_arrangement_helper_test.go, do you think it's worth adding tests for these new conditions?

[ChrisMcD1]

We have fancy tests for this kind of stuff in window_arrangement_helper_test.go, do you think it's worth adding tests for these new conditions?

Sure, just added! Those tests don't end up exercising the crash point that created this PR, so I also made an integration test for that behavior in particular.

I also realized I had inadvertently duplicated the logic of "make the main view full screen", so I added a fixup to simply that logic.

[stefanhaller]

Looks great. Just one last question about the integration test: I verified that it crashes when I revert the production code changes, but only if I run it with go test pkg/integration/clients/*.go. If I run it with go run cmd/integration_test/main.go cli ui/side_panel_width_1, then it succeeds. Any idea why that might be?

[ChrisMcD1]

Hmmm, I'm unable to reproduce that. If I cherrypick the test out to master, and run it with:

(cmd)chrismcdonnell@Chris-Laptop:~/linux-repos/lazygit$ go run cmd/integration_test/main.go cli ui/side_panel_width_1
panic: runtime error: index out of range [0] with length 0

goroutine 1 [running]:
github.com/jesseduffield/lazycore/pkg/boxlayout.normalizeWeights({0xc000414300, 0x2, 0x2})
        /home/chrismcdonnell/linux-repos/lazygit/vendor/github.com/jesseduffield/lazycore/pkg/boxlayout/boxlayout.go:161 +0x46f
github.com/jesseduffield/lazycore/pkg/boxlayout.calcSizes({0xc000426db0, 0x2, 0x0?}, 0xbe)
        /home/chrismcdonnell/linux-repos/lazygit/vendor/github.com/jesseduffield/lazycore/pkg/boxlayout/boxlayout.go:97 +0x65
github.com/jesseduffield/lazycore/pkg/boxlayout.ArrangeWindows(0xc0004662d0, 0x0, 0x0, 0xbe, 0x2a)

I get the error. I also squashed the 3 commits and reverted them, and then it also failed there

[stefanhaller]

I investigated a bit more, and found that it depends on the window geometry: if the window is twice as wide as it is tall, the test succeeds; if it is square, it crashes.

Playing around with it more, I'm not sure the behavior is always what we want when you are in staging or in patch building mode. I need more time to wrap my head around it, and probably won't get around to it this weekend; but I'd like to hold off merging this until I understand this better.

[stefanhaller]

Ok, so there are problems because of this code: it runs unconditionally no matter whether we are in half-screen or full-screen mode, and this causes weird behavior, depending on the window geometry. For example, if you select a directory in the files panel that has both staged and unstaged changes, then it will show the main panels even with sidePanelWidth: 1 if the window is wide enough (because if it is, we split the main panels side-by-side), or if you set gui.mainPanelSplitMode: horizontal explicitly. I suppose this is also the reason why that integration test failed or succeeded depending on how wide the window is.

I think this condition needs to move somehow so that it is only checked when we're not in full or half screen mode, and only if mainSectionWeight is not 1. I don't have enough energy right now to make a concrete suggestion what this could look like.

Other observations:

• with sidePanelWidth: 1, you can't see the command log panel even if you focus it. I guess it needs a similar condition as the main views when they are focused.
• with sidePanelWidth: 0, lazygit crashes on startup. Probably not something that anybody would do, but might still be nice to be graceful about it, somehow (maybe just by clamping to a known good minimal value).