Let OnUIThreadAndWait's error be about the wait, not about f

Every caller passes an f that unconditionally returns nil, so f's error
return has never carried anything: the value is dead weight, and it
occupies the one channel the wait itself needs to report that it couldn't
run f at all. Drop it, so that the error the wait returns can only ever
mean that.

Work that can fail hands its error back through a captured variable, the
way the background fetch already hands back four values, which keeps the
two outcomes distinguishable at a call site that has both.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

pkg/gocui/gui.go

@@ -893,36 +893,42 @@ func (g *Gui) EndBlockingEvents() error {
 }
 
 // OnUIThreadAndWait runs f on the main event-loop goroutine and blocks the
-// caller until f has run, returning f's error. Use it to read UI-thread-owned
-// state (the model, contexts) from a worker without racing the UI thread.
+// caller until f has run. Use it to read UI-thread-owned state (the model,
+// contexts) from a worker without racing the UI thread.
+//
+// The error it returns is the wait's own, never f's: it reports that f was not
+// run at all. f doesn't report an error because what callers want on the UI
+// thread — reading and mutating state — doesn't fail.
 //
 // It must be called from a worker goroutine, never from the UI thread itself:
 // the UI thread would block waiting for a callback only it can run, which
 // deadlocks. Callers arrange this by construction (see the refresh helper's
 // RefreshFromWorker); a debug-only assertion there guards against getting it
 // wrong.
-func (g *Gui) OnUIThreadAndWait(f func() error) error {
+func (g *Gui) OnUIThreadAndWait(f func()) error {
 	return g.onUIThreadAndWait(f, false)
 }
 
 // Like OnUIThreadAndWait, but the enqueued work belongs to a background routine,
 // so it doesn't count towards the program being busy (see UpdateBackground).
-func (g *Gui) OnUIThreadAndWaitBackground(f func() error) error {
+func (g *Gui) OnUIThreadAndWaitBackground(f func()) error {
 	return g.onUIThreadAndWait(f, true)
 }
 
-func (g *Gui) onUIThreadAndWait(f func() error, background bool) error {
+func (g *Gui) onUIThreadAndWait(f func(), background bool) error {
 	enqueue := g.Update
 	if background {
 		enqueue = g.UpdateBackground
 	}
 
-	result := make(chan error, 1)
+	ran := make(chan struct{})
 	enqueue(func(*Gui) error {
-		result <- f()
+		f()
+		close(ran)
 		return nil
 	})
-	return <-result
+	<-ran
+	return nil
 }
 
 // Calls a function in a goroutine. Handles panics gracefully and tracks

pkg/gui/background.go

@@ -119,13 +119,12 @@ func (self *BackgroundRoutineMgr) startBackgroundFetch() {
 		var appStatusHelper *helpers.AppStatusHelper
 		var branchesHelper *helpers.BranchesHelper
 		var fetchGeneration int
-		if err := self.gui.g.OnUIThreadAndWaitBackground(func() error {
+		if err := self.gui.g.OnUIThreadAndWaitBackground(func() {
 			git = self.gui.git
 			appStatusHelper = self.gui.helpers.AppStatus
 			branchesHelper = self.gui.helpers.BranchesHelper
 			fetchGeneration = self.gui.c.State().GetRepoGeneration()
 			self.gui.State.LastBackgroundFetchTime = time.Now()
-			return nil
 		}); err != nil {
 			return err
 		}
@@ -184,10 +183,9 @@ func (self *BackgroundRoutineMgr) checkForExternalChanges() {
 	// reading them from this background goroutine would race the reassignment.
 	var git *commands.GitCommand
 	var refreshHelper *helpers.RefreshHelper
-	if err := self.gui.g.OnUIThreadAndWaitBackground(func() error {
+	if err := self.gui.g.OnUIThreadAndWaitBackground(func() {
 		git = self.gui.git
 		refreshHelper = self.gui.helpers.Refresh
-		return nil
 	}); err != nil {
 		return
 	}

pkg/gui/controllers/helpers/merge_and_rebase_helper.go

@@ -188,9 +188,8 @@ func (self *MergeAndRebaseHelper) hasExecTodos(calledFromWorker bool) bool {
 	}
 
 	result := false
-	_ = self.c.GocuiGui().OnUIThreadAndWait(func() error {
+	_ = self.c.GocuiGui().OnUIThreadAndWait(func() {
 		result = check()
-		return nil
 	})
 	return result
 }

pkg/gui/controllers/helpers/refresh_helper.go

@@ -1254,14 +1254,10 @@ func (self *RefreshHelper) captureOnUIThread(calledFromWorker bool, background b
 		return
 	}
 
-	wrapped := func() error {
-		fn()
-		return nil
-	}
 	if background {
-		_ = self.c.GocuiGui().OnUIThreadAndWaitBackground(wrapped)
+		_ = self.c.GocuiGui().OnUIThreadAndWaitBackground(fn)
 	} else {
-		_ = self.c.GocuiGui().OnUIThreadAndWait(wrapped)
+		_ = self.c.GocuiGui().OnUIThreadAndWait(fn)
 	}
 }
 

pkg/gui/controllers/patch_building_controller.go

@@ -230,9 +230,8 @@ func (self *PatchBuildingController) discardSelectionFromCommit() error {
 		err := self.c.Git().Patch.DeletePatchesFromCommit(commits, commitIndex)
 		// Escape pops the patch-building context, so run it on the UI thread
 		// before the refresh below.
-		_ = self.c.GocuiGui().OnUIThreadAndWait(func() error {
+		_ = self.c.GocuiGui().OnUIThreadAndWait(func() {
 			self.c.Helpers().PatchBuilding.Escape()
-			return nil
 		})
 		return self.c.Helpers().MergeAndRebase.CheckMergeOrRebaseWithRefreshOptions(
 			err, types.RefreshOptions{})

pkg/gui/gui_driver.go

@@ -82,7 +82,7 @@ func (self *GuiDriver) WaitUntilIdle() {
 }
 
 func (self *GuiDriver) OnUIThreadAndWait(f func()) {
-	_ = self.gui.g.OnUIThreadAndWait(func() error { f(); return nil })
+	_ = self.gui.g.OnUIThreadAndWait(f)
 }
 
 func (self *GuiDriver) replayMouseEvent(x, y int, buttons tcell.ButtonMask) {

pkg/gui/tasks_adapter.go

@@ -80,9 +80,8 @@ func (gui *Gui) newStringTaskWithoutScroll(view *gocui.View, str string) error {
 	manager := gui.getManager(view)
 
 	f := func(tasks.TaskOpts) error {
-		return gui.g.OnUIThreadAndWaitBackground(func() error {
+		return gui.g.OnUIThreadAndWaitBackground(func() {
 			gui.c.SetViewContent(view, str)
-			return nil
 		})
 	}
 
@@ -97,10 +96,9 @@ func (gui *Gui) newStringTaskWithScroll(view *gocui.View, str string, originX in
 	manager := gui.getManager(view)
 
 	f := func(tasks.TaskOpts) error {
-		return gui.g.OnUIThreadAndWaitBackground(func() error {
+		return gui.g.OnUIThreadAndWaitBackground(func() {
 			gui.c.SetViewContent(view, str)
 			view.SetOrigin(originX, originY)
-			return nil
 		})
 	}
 
@@ -115,10 +113,9 @@ func (gui *Gui) newStringTaskWithKey(view *gocui.View, str string, key string) e
 	manager := gui.getManager(view)
 
 	f := func(tasks.TaskOpts) error {
-		return gui.g.OnUIThreadAndWaitBackground(func() error {
+		return gui.g.OnUIThreadAndWaitBackground(func() {
 			gui.c.ResetViewOrigin(view)
 			gui.c.SetViewContent(view, str)
-			return nil
 		})
 	}
 

pkg/tasks/tasks.go

@@ -87,7 +87,7 @@ type ViewBufferManager struct {
 	// of the view happen through this, so that the view is only ever touched on
 	// the UI thread (where it is also laid out and drawn), never on the task's
 	// own goroutine.
-	onUIThread func(f func() error) error
+	onUIThread func(f func()) error
 
 	// if the user flicks through a heap of items, with each one
 	// spawning a process to render something to the main view,
@@ -126,7 +126,7 @@ func NewViewBufferManager(
 	onEndOfInput func(),
 	onNewKey func(),
 	newGocuiTask func() gocui.Task,
-	onUIThread func(f func() error) error,
+	onUIThread func(f func()) error,
 ) *ViewBufferManager {
 	return &ViewBufferManager{
 		Log:          log,
@@ -358,10 +358,7 @@ func (self *ViewBufferManager) NewCmdTask(start func() (Cmd, io.Reader), prefix
 							// onEndOfInput reads the view's dimensions (to decide
 							// whether to scroll) and sets the origin, both of which
 							// are UI-thread-only, so run it there.
-							_ = self.onUIThread(func() error {
-								self.onEndOfInput()
-								return nil
-							})
+							_ = self.onUIThread(self.onEndOfInput)
 							callThen()
 							break outer
 						}
@@ -502,10 +499,7 @@ func (self *ViewBufferManager) NewTask(f func(TaskOpts) error, key string) error
 			// must happen after releasing taskIDMutex: it blocks until the UI
 			// thread runs it, and a NewTask call on the UI thread takes
 			// taskIDMutex, so holding it here would deadlock.
-			_ = self.onUIThread(func() error {
-				self.onNewKey()
-				return nil
-			})
+			_ = self.onUIThread(self.onNewKey)
 		}
 
 		self.waitingMutex.Lock()

pkg/tasks/tasks_test.go

@@ -40,7 +40,7 @@ func TestNewCmdTaskInstantStop(t *testing.T) {
 		onNewKey,
 		newTask,
 		// no UI thread in the test; run the view mutations inline
-		func(f func() error) error { return f() },
+		func(f func()) error { f(); return nil },
 	)
 
 	stop := make(chan struct{})
@@ -107,7 +107,7 @@ func TestNewCmdTask(t *testing.T) {
 		onNewKey,
 		newTask,
 		// no UI thread in the test; run the view mutations inline
-		func(f func() error) error { return f() },
+		func(f func()) error { f(); return nil },
 	)
 
 	stop := make(chan struct{})
@@ -242,7 +242,7 @@ func TestNewCmdTaskRefresh(t *testing.T) {
 			func() {},
 			newTask,
 			// no UI thread in the test; run the view mutations inline
-			func(f func() error) error { return f() },
+			func(f func()) error { f(); return nil },
 		)
 
 		stop := make(chan struct{})