Properly fix accordion issue (#2792)

go.mod

@@ -18,7 +18,7 @@ require (
 	github.com/integrii/flaggy v1.4.0
 	github.com/jesseduffield/generics v0.0.0-20220320043834-727e535cbe68
 	github.com/jesseduffield/go-git/v5 v5.1.2-0.20221018185014-fdd53fef665d
-	github.com/jesseduffield/gocui v0.3.1-0.20230719103719-ea5c8b64cf35
+	github.com/jesseduffield/gocui v0.3.1-0.20230719120401-398f4965241f
 	github.com/jesseduffield/kill v0.0.0-20220618033138-bfbe04675d10
 	github.com/jesseduffield/lazycore v0.0.0-20221012050358-03d2e40243c5
 	github.com/jesseduffield/minimal/gitignore v0.3.3-0.20211018110810-9cde264e6b1e

go.sum

@@ -72,8 +72,8 @@ github.com/jesseduffield/generics v0.0.0-20220320043834-727e535cbe68 h1:EQP2Tv8T
 github.com/jesseduffield/generics v0.0.0-20220320043834-727e535cbe68/go.mod h1:+LLj9/WUPAP8LqCchs7P+7X0R98HiFujVFANdNaxhGk=
 github.com/jesseduffield/go-git/v5 v5.1.2-0.20221018185014-fdd53fef665d h1:bO+OmbreIv91rCe8NmscRwhFSqkDJtzWCPV4Y+SQuXE=
 github.com/jesseduffield/go-git/v5 v5.1.2-0.20221018185014-fdd53fef665d/go.mod h1:nGNEErzf+NRznT+N2SWqmHnDnF9aLgANB1CUNEan09o=
-github.com/jesseduffield/gocui v0.3.1-0.20230719103719-ea5c8b64cf35 h1:ZWI/Tkg89iTpZDuxWYsR/f6xu+U827N8L1YnSvqug88=
-github.com/jesseduffield/gocui v0.3.1-0.20230719103719-ea5c8b64cf35/go.mod h1:dJ/BEUt3OWtaRg/PmuJWendRqREhre9JQ1SLvqrVJ8s=
+github.com/jesseduffield/gocui v0.3.1-0.20230719120401-398f4965241f h1:w/pxI34XepTAx4HwxUu8ipimbVRgSTS+7ahmgFQwH80=
+github.com/jesseduffield/gocui v0.3.1-0.20230719120401-398f4965241f/go.mod h1:dJ/BEUt3OWtaRg/PmuJWendRqREhre9JQ1SLvqrVJ8s=
 github.com/jesseduffield/kill v0.0.0-20220618033138-bfbe04675d10 h1:jmpr7KpX2+2GRiE91zTgfq49QvgiqB0nbmlwZ8UnOx0=
 github.com/jesseduffield/kill v0.0.0-20220618033138-bfbe04675d10/go.mod h1:aA97kHeNA+sj2Hbki0pvLslmE4CbDyhBeSSTUUnOuVo=
 github.com/jesseduffield/lazycore v0.0.0-20221012050358-03d2e40243c5 h1:CDuQmfOjAtb1Gms6a1p5L2P8RhbLUq5t8aL7PiQd2uY=

pkg/gui/context/list_context_trait.go

@@ -31,7 +31,14 @@ func (self *ListContextTrait) GetList() types.IList {
 }
 
 func (self *ListContextTrait) FocusLine() {
-	self.GetViewTrait().FocusPoint(self.list.GetSelectedLineIdx())
+	// Doing this at the end of the layout function because we need the view to be
+	// resized before we focus the line, otherwise if we're in accordion mode
+	// the view could be squashed and won't how to adjust the cursor/origin
+	self.c.AfterLayout(func() error {
+		self.GetViewTrait().FocusPoint(self.list.GetSelectedLineIdx())
+		return nil
+	})
+
 	self.setFooter()
 
 	if self.refreshViewportOnChange {

pkg/gui/gui.go

@@ -132,6 +132,8 @@ type Gui struct {
 	helpers *helpers.Helpers
 
 	integrationTest integrationTypes.IntegrationTest
+
+	afterLayoutFuncs chan func() error
 }
 
 type StateAccessor struct {
@@ -458,7 +460,8 @@ func NewGui(
 			PopupMutex:              &deadlock.Mutex{},
 			PtyMutex:                &deadlock.Mutex{},
 		},
-		InitialDir: initialDir,
+		InitialDir:       initialDir,
+		afterLayoutFuncs: make(chan func() error, 1000),
 	}
 
 	gui.WatchFilesForChanges()
@@ -519,9 +522,29 @@ var RuneReplacements = map[rune]string{
 }
 
 func (gui *Gui) initGocui(headless bool, test integrationTypes.IntegrationTest) (*gocui.Gui, error) {
-	playRecording := test != nil && os.Getenv(components.SANDBOX_ENV_VAR) != "true"
+	runInSandbox := os.Getenv(components.SANDBOX_ENV_VAR) == "true"
+	playRecording := test != nil && !runInSandbox
 
-	g, err := gocui.NewGui(gocui.OutputTrue, OverlappingEdges, playRecording, headless, RuneReplacements)
+	width, height := 0, 0
+	if test != nil {
+		if test.RequiresHeadless() {
+			if runInSandbox {
+				panic("Test requires headless, can't run in sandbox")
+			}
+			headless = true
+		}
+		width, height = test.HeadlessDimensions()
+	}
+
+	g, err := gocui.NewGui(gocui.NewGuiOpts{
+		OutputMode:       gocui.OutputTrue,
+		SupportOverlaps:  OverlappingEdges,
+		PlayRecording:    playRecording,
+		Headless:         headless,
+		RuneReplacements: RuneReplacements,
+		Width:            width,
+		Height:           height,
+	})
 	if err != nil {
 		return nil, err
 	}

pkg/gui/gui_common.go

@@ -168,3 +168,12 @@ func (self *guiCommon) IsAnyModeActive() bool {
 func (self *guiCommon) GetInitialKeybindingsWithCustomCommands() ([]*types.Binding, []*gocui.ViewMouseBinding) {
 	return self.gui.GetInitialKeybindingsWithCustomCommands()
 }
+
+func (self *guiCommon) AfterLayout(f func() error) {
+	select {
+	case self.gui.afterLayoutFuncs <- f:
+	default:
+		// hopefully this never happens
+		self.gui.c.Log.Error("afterLayoutFuncs channel is full, skipping function")
+	}
+}

pkg/gui/layout.go

@@ -149,7 +149,23 @@ func (gui *Gui) layout(g *gocui.Gui) error {
 	// if you run `lazygit --logs`
 	// this will let you see these branches as prettified json
 	// gui.c.Log.Info(utils.AsJson(gui.State.Model.Branches[0:4]))
-	return gui.helpers.Confirmation.ResizeCurrentPopupPanel()
+	if err := gui.helpers.Confirmation.ResizeCurrentPopupPanel(); err != nil {
+		return err
+	}
+
+outer:
+	for {
+		select {
+		case f := <-gui.afterLayoutFuncs:
+			if err := f(); err != nil {
+				return err
+			}
+		default:
+			break outer
+		}
+	}
+
+	return nil
 }
 
 func (gui *Gui) prepareView(viewName string) (*gocui.View, error) {

pkg/gui/types/common.go

@@ -80,6 +80,10 @@ type IGuiCommon interface {
 	// Runs a function in a goroutine. Use this whenever you want to run a goroutine and keep track of the fact
 	// that lazygit is still busy. See docs/dev/Busy.md
 	OnWorker(f func(gocui.Task))
+	// Function to call at the end of our 'layout' function which renders views
+	// For example, you may want a view's line to be focused only after that view is
+	// resized, if in accordion mode.
+	AfterLayout(f func() error)
 
 	// returns the gocui Gui struct. There is a good chance you don't actually want to use
 	// this struct and instead want to use another method above

pkg/integration/clients/tui.go

@@ -28,7 +28,10 @@ func RunTUI() {
 	app := newApp(testDir)
 	app.loadTests()
 
-	g, err := gocui.NewGui(gocui.OutputTrue, false, false, false, gui.RuneReplacements)
+	g, err := gocui.NewGui(gocui.NewGuiOpts{
+		OutputMode:       gocui.OutputTrue,
+		RuneReplacements: gui.RuneReplacements,
+	})
 	if err != nil {
 		log.Panicln(err)
 	}

pkg/integration/components/test.go

@@ -19,6 +19,11 @@ import (
 // to get the test's name via it's file's path.
 const unitTestDescription = "test test"
 
+const (
+	defaultWidth  = 100
+	defaultHeight = 100
+)
+
 type IntegrationTest struct {
 	name         string
 	description  string
@@ -32,6 +37,8 @@ type IntegrationTest struct {
 		keys config.KeybindingConfig,
 	)
 	gitVersion GitVersionRestriction
+	width      int
+	height     int
 }
 
 var _ integrationTypes.IntegrationTest = &IntegrationTest{}
@@ -52,6 +59,11 @@ type NewIntegrationTestArgs struct {
 	Skip         bool
 	// to run a test only on certain git versions
 	GitVersion GitVersionRestriction
+	// width and height when running in headless mode, for testing
+	// the UI in different sizes.
+	// If these are set, the test must be run in headless mode
+	Width  int
+	Height int
 }
 
 type GitVersionRestriction struct {
@@ -120,6 +132,8 @@ func NewIntegrationTest(args NewIntegrationTestArgs) *IntegrationTest {
 		setupConfig:  args.SetupConfig,
 		run:          args.Run,
 		gitVersion:   args.GitVersion,
+		width:        args.Width,
+		height:       args.Height,
 	}
 }
 
@@ -172,6 +186,18 @@ func (self *IntegrationTest) Run(gui integrationTypes.GuiDriver) {
 	}
 }
 
+func (self *IntegrationTest) HeadlessDimensions() (int, int) {
+	if self.width == 0 && self.height == 0 {
+		return defaultWidth, defaultHeight
+	}
+
+	return self.width, self.height
+}
+
+func (self *IntegrationTest) RequiresHeadless() bool {
+	return self.width != 0 && self.height != 0
+}
+
 func testNameFromCurrentFilePath() string {
 	path := utils.FilePath(3)
 	return TestNameFromFilePath(path)

pkg/integration/components/view_driver.go

@@ -82,6 +82,20 @@ func (self *ViewDriver) TopLines(matchers ...*TextMatcher) *ViewDriver {
 	return self.assertLines(0, matchers...)
 }
 
+// Asserts on the visible lines of the view.
+// Note, this assumes that the view's viewport is filled with lines
+func (self *ViewDriver) VisibleLines(matchers ...*TextMatcher) *ViewDriver {
+	self.validateMatchersPassed(matchers)
+	self.validateVisibleLineCount(matchers)
+
+	// Get the origin of the view and offset that.
+	// Note that we don't do any retrying here so if we want to bring back retry logic
+	// we'll need to update this.
+	originY := self.getView().OriginY()
+
+	return self.assertLines(originY, matchers...)
+}
+
 // asserts that somewhere in the view there are consequetive lines matching the given matchers.
 func (self *ViewDriver) ContainsLines(matchers ...*TextMatcher) *ViewDriver {
 	self.validateMatchersPassed(matchers)
@@ -212,6 +226,16 @@ func (self *ViewDriver) validateEnoughLines(matchers []*TextMatcher) {
 	})
 }
 
+// assumes the view's viewport is filled with lines
+func (self *ViewDriver) validateVisibleLineCount(matchers []*TextMatcher) {
+	view := self.getView()
+
+	self.t.assertWithRetries(func() (bool, string) {
+		count := view.InnerHeight() + 1
+		return count == len(matchers), fmt.Sprintf("unexpected number of visible lines in view '%s'. Expected exactly %d, got %d", view.Name(), len(matchers), count)
+	})
+}
+
 func (self *ViewDriver) assertLines(offset int, matchers ...*TextMatcher) *ViewDriver {
 	view := self.getView()
 

pkg/integration/tests/test_list.go

@@ -209,6 +209,7 @@ var tests = []*components.IntegrationTest{
 	tag.CrudAnnotated,
 	tag.CrudLightweight,
 	tag.Reset,
+	ui.Accordion,
 	ui.DoublePopup,
 	ui.SwitchTabFromMenu,
 	undo.UndoCheckoutAndDrop,

pkg/integration/tests/ui/accordion.go

@@ -0,0 +1,61 @@
+package ui
+
+import (
+	"github.com/jesseduffield/lazygit/pkg/config"
+	. "github.com/jesseduffield/lazygit/pkg/integration/components"
+)
+
+// When in acccordion mode, Lazygit looks like this:
+//
+// ╶─Status─────────────────────────╴┌─Patch──────────────────────────────────────────────────────────┐
+// ╶─Files - Submodules──────0 of 0─╴│commit 6e56dd04b70e548976f7f2928c4d9c359574e2bc                 ▲
+// ╶─Local branches - Remotes1 of 1─╴│Author: CI <CI@example.com>                                     █
+// ┌─Commits - Reflog───────────────┐│Date:   Wed Jul 19 22:00:03 2023 +1000                          │
+// │7fe02805 CI commit 12           ▲│                                                                ▼
+// │6e56dd04 CI commit 11           █└────────────────────────────────────────────────────────────────┘
+// │a35c687d CI commit 10           ▼┌─Command log────────────────────────────────────────────────────┐
+// └───────────────────────10 of 20─┘│Random tip: To filter commits by path, press '<c-s>'            │
+// ╶─Stash───────────────────0 of 0─╴└────────────────────────────────────────────────────────────────┘
+//  <pgup>/<pgdown>: Scroll, <esc>: Cancel, q: Quit, ?: Keybindings, 1-Donate Ask Question unversioned
+
+var Accordion = NewIntegrationTest(NewIntegrationTestArgs{
+	Description:  "Verify accordion mode kicks in when the screen height is too small",
+	ExtraCmdArgs: []string{},
+	Width:        100,
+	Height:       10,
+	Skip:         false,
+	SetupConfig:  func(config *config.AppConfig) {},
+	SetupRepo: func(shell *Shell) {
+		shell.CreateNCommits(20)
+	},
+	Run: func(t *TestDriver, keys config.KeybindingConfig) {
+		t.Views().Commits().
+			Focus().
+			VisibleLines(
+				Contains("commit 20").IsSelected(),
+				Contains("commit 19"),
+				Contains("commit 18"),
+			).
+			// go past commit 11, then come back, so that it ends up in the centre of the viewport
+			NavigateToLine(Contains("commit 11")).
+			NavigateToLine(Contains("commit 10")).
+			NavigateToLine(Contains("commit 11")).
+			VisibleLines(
+				Contains("commit 12"),
+				Contains("commit 11").IsSelected(),
+				Contains("commit 10"),
+			)
+
+		t.Views().Files().
+			Focus()
+
+		// ensure we retain the same viewport upon re-focus
+		t.Views().Commits().
+			Focus().
+			VisibleLines(
+				Contains("commit 12"),
+				Contains("commit 11").IsSelected(),
+				Contains("commit 10"),
+			)
+	},
+})

pkg/integration/types/types.go

@@ -13,6 +13,9 @@ import (
 type IntegrationTest interface {
 	Run(GuiDriver)
 	SetupConfig(config *config.AppConfig)
+	RequiresHeadless() bool
+	// width and height when running headless
+	HeadlessDimensions() (int, int)
 }
 
 // this is the interface through which our integration tests interact with the lazygit gui

vendor/github.com/jesseduffield/gocui/gui.go

@@ -177,13 +177,26 @@ type Gui struct {
 	taskManager *TaskManager
 }
 
+type NewGuiOpts struct {
+	OutputMode      OutputMode
+	SupportOverlaps bool
+	PlayRecording   bool
+	Headless        bool
+	// only applicable when Headless is true
+	Width int
+	// only applicable when Headless is true
+	Height int
+
+	RuneReplacements map[rune]string
+}
+
 // NewGui returns a new Gui object with a given output mode.
-func NewGui(mode OutputMode, supportOverlaps bool, playRecording bool, headless bool, runeReplacements map[rune]string) (*Gui, error) {
+func NewGui(opts NewGuiOpts) (*Gui, error) {
 	g := &Gui{}
 
 	var err error
-	if headless {
-		err = g.tcellInitSimulation()
+	if opts.Headless {
+		err = g.tcellInitSimulation(opts.Width, opts.Height)
 	} else {
 		err = g.tcellInit(runeReplacements)
 	}
@@ -191,7 +204,7 @@ func NewGui(mode OutputMode, supportOverlaps bool, playRecording bool, headless
 		return nil, err
 	}
 
-	if headless || runtime.GOOS == "windows" {
+	if opts.Headless || runtime.GOOS == "windows" {
 		g.maxX, g.maxY = g.screen.Size()
 	} else {
 		// TODO: find out if we actually need this bespoke logic for linux
@@ -201,7 +214,7 @@ func NewGui(mode OutputMode, supportOverlaps bool, playRecording bool, headless
 		}
 	}
 
-	g.outputMode = mode
+	g.outputMode = opts.OutputMode
 
 	g.stop = make(chan struct{})
 
@@ -209,7 +222,7 @@ func NewGui(mode OutputMode, supportOverlaps bool, playRecording bool, headless
 	g.userEvents = make(chan userEvent, 20)
 	g.taskManager = newTaskManager()
 
-	if playRecording {
+	if opts.PlayRecording {
 		g.ReplayedEvents = replayedEvents{
 			Keys:    make(chan *TcellKeyEventWrapper),
 			Resizes: make(chan *TcellResizeEventWrapper),
@@ -221,14 +234,14 @@ func NewGui(mode OutputMode, supportOverlaps bool, playRecording bool, headless
 
 	// SupportOverlaps is true when we allow for view edges to overlap with other
 	// view edges
-	g.SupportOverlaps = supportOverlaps
+	g.SupportOverlaps = opts.SupportOverlaps
 
 	// default keys for when searching strings in a view
 	g.SearchEscapeKey = KeyEsc
 	g.NextSearchMatchKey = 'n'
 	g.PrevSearchMatchKey = 'N'
 
-	g.playRecording = playRecording
+	g.playRecording = opts.PlayRecording
 
 	return g, nil
 }

vendor/github.com/jesseduffield/gocui/tcell_driver.go

@@ -81,7 +81,7 @@ func registerRuneFallbacks(s tcell.Screen, additional map[rune]string) {
 }
 
 // tcellInitSimulation initializes tcell screen for use.
-func (g *Gui) tcellInitSimulation() error {
+func (g *Gui) tcellInitSimulation(width int, height int) error {
 	s := tcell.NewSimulationScreen("")
 	if e := s.Init(); e != nil {
 		return e
@@ -90,7 +90,7 @@ func (g *Gui) tcellInitSimulation() error {
 		Screen = s
 		// setting to a larger value than the typical terminal size
 		// so that during a test we're more likely to see an item to select in a view.
-		s.SetSize(100, 100)
+		s.SetSize(width, height)
 		s.Sync()
 		return nil
 	}

vendor/modules.txt

@@ -172,7 +172,7 @@ github.com/jesseduffield/go-git/v5/utils/merkletrie/filesystem
 github.com/jesseduffield/go-git/v5/utils/merkletrie/index
 github.com/jesseduffield/go-git/v5/utils/merkletrie/internal/frame
 github.com/jesseduffield/go-git/v5/utils/merkletrie/noder
-# github.com/jesseduffield/gocui v0.3.1-0.20230719103719-ea5c8b64cf35
+# github.com/jesseduffield/gocui v0.3.1-0.20230719120401-398f4965241f
 ## explicit; go 1.12
 github.com/jesseduffield/gocui
 # github.com/jesseduffield/kill v0.0.0-20220618033138-bfbe04675d10