From d2d13449e498017c2a327ee6a95b29ed2a23d664 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Fri, 28 Mar 2025 11:06:35 +0100 Subject: [PATCH 1/2] Cleanup: remove pointless `if` statement --- pkg/gui/pty.go | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/pkg/gui/pty.go b/pkg/gui/pty.go index 4fcfdbe0e..3341c1e3c 100644 --- a/pkg/gui/pty.go +++ b/pkg/gui/pty.go @@ -90,11 +90,7 @@ func (gui *Gui) newPtyTask(view *gocui.View, cmd *exec.Cmd, prefix string) error } linesToRead := gui.linesToReadFromCmdTask(view) - if err := manager.NewTask(manager.NewCmdTask(start, prefix, linesToRead, onClose), cmdStr); err != nil { - return err - } - - return nil + return manager.NewTask(manager.NewCmdTask(start, prefix, linesToRead, onClose), cmdStr) } func removeExistingTermEnvVars(env []string) []string { From 10f29bc6b481b4d6c1f0517f8845fd1f2c375767 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Fri, 28 Mar 2025 11:05:44 +0100 Subject: [PATCH 2/2] Fix race with PTYs in integration tests In 8b8343b8a9f we made a change to run newPtyTask from AfterLayout; this is needed so that the PTY gets the new, updated view size. However, this created a race condition for integration tests that select a line in a list view and then expect the main view to have certain content; sometimes that content gets rendered too late. I'm surprised that this didn't cause more tests to fail; right now I only know of one test that occasionally fails because of this, which is stash/rename.go. Fix this by moving the AfterLayout to inside newPtyTask, and do it only when we are actually using a PTY (we don't when no pager is configured, which is the case for integration tests). The diff is best viewed with "ignore whitespace" turned on. --- pkg/gui/main_panels.go | 5 +--- pkg/gui/pty.go | 68 ++++++++++++++++++++++++------------------ 2 files changed, 40 insertions(+), 33 deletions(-) diff --git a/pkg/gui/main_panels.go b/pkg/gui/main_panels.go index 54e3e1c0c..30055e805 100644 --- a/pkg/gui/main_panels.go +++ b/pkg/gui/main_panels.go @@ -20,10 +20,7 @@ func (gui *Gui) runTaskForView(view *gocui.View, task types.UpdateTask) error { return gui.newCmdTask(view, v.Cmd, v.Prefix) case *types.RunPtyTask: - gui.afterLayout(func() error { - return gui.newPtyTask(view, v.Cmd, v.Prefix) - }) - return nil + return gui.newPtyTask(view, v.Cmd, v.Prefix) } return nil diff --git a/pkg/gui/pty.go b/pkg/gui/pty.go index 3341c1e3c..84c9e6da8 100644 --- a/pkg/gui/pty.go +++ b/pkg/gui/pty.go @@ -54,43 +54,53 @@ func (gui *Gui) newPtyTask(view *gocui.View, cmd *exec.Cmd, prefix string) error return gui.newCmdTask(view, cmd, prefix) } - cmdStr := strings.Join(cmd.Args, " ") + // Run the pty after layout so that it gets the correct size + gui.afterLayout(func() error { + // Need to get the width and the pager again because the layout might have + // changed the size of the view + width = view.InnerWidth() + pager = gui.git.Config.GetPager(width) - // This communicates to pagers that we're in a very simple - // terminal that they should not expect to have much capabilities. - // Moving the cursor, clearing the screen, or querying for colors are among such "advanced" capabilities. - // Context: https://github.com/jesseduffield/lazygit/issues/3419 - cmd.Env = removeExistingTermEnvVars(cmd.Env) - cmd.Env = append(cmd.Env, "TERM=dumb") + cmdStr := strings.Join(cmd.Args, " ") - cmd.Env = append(cmd.Env, "GIT_PAGER="+pager) + // This communicates to pagers that we're in a very simple + // terminal that they should not expect to have much capabilities. + // Moving the cursor, clearing the screen, or querying for colors are among such "advanced" capabilities. + // Context: https://github.com/jesseduffield/lazygit/issues/3419 + cmd.Env = removeExistingTermEnvVars(cmd.Env) + cmd.Env = append(cmd.Env, "TERM=dumb") - manager := gui.getManager(view) + cmd.Env = append(cmd.Env, "GIT_PAGER="+pager) - var ptmx *os.File - start := func() (*exec.Cmd, io.Reader) { - var err error - ptmx, err = pty.StartWithSize(cmd, gui.desiredPtySize(view)) - if err != nil { - gui.c.Log.Error(err) + manager := gui.getManager(view) + + var ptmx *os.File + start := func() (*exec.Cmd, io.Reader) { + var err error + ptmx, err = pty.StartWithSize(cmd, gui.desiredPtySize(view)) + if err != nil { + gui.c.Log.Error(err) + } + + gui.Mutexes.PtyMutex.Lock() + gui.viewPtmxMap[view.Name()] = ptmx + gui.Mutexes.PtyMutex.Unlock() + + return cmd, ptmx } - gui.Mutexes.PtyMutex.Lock() - gui.viewPtmxMap[view.Name()] = ptmx - gui.Mutexes.PtyMutex.Unlock() + onClose := func() { + gui.Mutexes.PtyMutex.Lock() + ptmx.Close() + delete(gui.viewPtmxMap, view.Name()) + gui.Mutexes.PtyMutex.Unlock() + } - return cmd, ptmx - } + linesToRead := gui.linesToReadFromCmdTask(view) + return manager.NewTask(manager.NewCmdTask(start, prefix, linesToRead, onClose), cmdStr) + }) - onClose := func() { - gui.Mutexes.PtyMutex.Lock() - ptmx.Close() - delete(gui.viewPtmxMap, view.Name()) - gui.Mutexes.PtyMutex.Unlock() - } - - linesToRead := gui.linesToReadFromCmdTask(view) - return manager.NewTask(manager.NewCmdTask(start, prefix, linesToRead, onClose), cmdStr) + return nil } func removeExistingTermEnvVars(env []string) []string {