Skip to content

Commit

Permalink
Simplify error handling (#3502)
Browse files Browse the repository at this point in the history 
- **PR Description**

Simplify and canonicalize error handling across the code base.

Previously it was important to make sure that errors don't bubble up
into gocui, because it would panic there; so we had to show errors in
error panels in the right places (in controller code, usually). This is
error-prone because it's easy to forget (see #3490 for an example);
also, it's not always obvious where in the call chain the error panel
needs to be shown. Most of the time it's in controller code, but we had
plenty of calls to `Error()` in helpers, and I remember that I found
this very confusing when I was new to the code base.

Change this so that you can simply return errors everywhere. The
functions to show an error message manually have been removed for
clarity.

I tried to structure the commits so that you can review them one by one.

Closes #3491.
  • Loading branch information
@stefanhaller
stefanhaller committed Apr 18, 2024
2 parents 145795c + caad553 commit 34f8f72
Show file tree
Hide file tree
Showing 76 changed files with 706 additions and 422 deletions.
8 changes: 4 additions & 4 deletions go.mod
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,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.20240309085756-86e0d5a312de
github.com/jesseduffield/gocui v0.3.1-0.20240418080333-8cd33929c513
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
Expand Down Expand Up @@ -47,7 +47,7 @@ require (
github.com/davecgh/go-spew v1.1.1 // indirect
github.com/emirpasic/gods v1.12.0 // indirect
github.com/fatih/color v1.9.0 // indirect
github.com/gdamore/encoding v1.0.0 // indirect
github.com/gdamore/encoding v1.0.1 // indirect
github.com/go-git/gcfg v1.5.0 // indirect
github.com/go-git/go-billy/v5 v5.0.0 // indirect
github.com/go-logfmt/logfmt v0.5.0 // indirect
Expand All @@ -73,8 +73,8 @@ require (
github.com/xanzy/ssh-agent v0.2.1 // indirect
golang.org/x/crypto v0.0.0-20220722155217-630584e8d5aa // indirect
golang.org/x/net v0.7.0 // indirect
golang.org/x/sys v0.18.0 // indirect
golang.org/x/term v0.18.0 // indirect
golang.org/x/sys v0.19.0 // indirect
golang.org/x/term v0.19.0 // indirect
golang.org/x/text v0.14.0 // indirect
gopkg.in/warnings.v0 v0.1.2 // indirect
)
15 changes: 8 additions & 7 deletions go.sum
View file
Original file line number Diff line number Diff line change
Expand Up @@ -87,8 +87,9 @@ github.com/flynn/go-shlex v0.0.0-20150515145356-3f9db97f8568/go.mod h1:xEzjJPgXI
github.com/fsmiamoto/git-todo-parser v0.0.5 h1:Bhzd/vz/6Qm3udfkd6NO9fWfD3TpwR9ucp3N75/J5I8=
github.com/fsmiamoto/git-todo-parser v0.0.5/go.mod h1:B+AgTbNE2BARvJqzXygThzqxLIaEWvwr2sxKYYb0Fas=
github.com/fsnotify/fsnotify v1.4.7/go.mod h1:jwhsz4b93w/PPRr/qN1Yymfu8t87LnFCMoQvtojpjFo=
github.com/gdamore/encoding v1.0.0 h1:+7OoQ1Bc6eTm5niUzBa0Ctsh6JbMW6Ra+YNuAtDBdko=
github.com/gdamore/encoding v1.0.0/go.mod h1:alR0ol34c49FCSBLjhosxzcPHQbf2trDkoo5dl+VrEg=
github.com/gdamore/encoding v1.0.1 h1:YzKZckdBL6jVt2Gc+5p82qhrGiqMdG/eNs6Wy0u3Uhw=
github.com/gdamore/encoding v1.0.1/go.mod h1:0Z0cMFinngz9kS1QfMjCP8TY7em3bZYeeklsSDPivEo=
github.com/gdamore/tcell/v2 v2.7.4 h1:sg6/UnTM9jGpZU+oFYAsDahfchWAFW8Xx2yFinNSAYU=
github.com/gdamore/tcell/v2 v2.7.4/go.mod h1:dSXtXTSK0VsW1biw65DZLZ2NKr7j0qP/0J7ONmsraWg=
github.com/gliderlabs/ssh v0.2.2 h1:6zsha5zo/TWhRhwqCD3+EarCAgZ2yN28ipRnGPnwkI0=
Expand Down Expand Up @@ -187,8 +188,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.20240309085756-86e0d5a312de h1:2ww1SWgakihE8hFxZ7L3agVeGpA6qwW5vdnhFUXKMQo=
github.com/jesseduffield/gocui v0.3.1-0.20240309085756-86e0d5a312de/go.mod h1:XtEbqCbn45keRXEu+OMZkjN5gw6AEob59afsgHjokZ8=
github.com/jesseduffield/gocui v0.3.1-0.20240418080333-8cd33929c513 h1:Y1bw5iItrsDCumATc/rklIJ/6K+68ieiWZJedhrNuXo=
github.com/jesseduffield/gocui v0.3.1-0.20240418080333-8cd33929c513/go.mod h1:XtEbqCbn45keRXEu+OMZkjN5gw6AEob59afsgHjokZ8=
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=
Expand Down Expand Up @@ -469,14 +470,14 @@ golang.org/x/sys v0.0.0-20220520151302-bc2c85ada10a/go.mod h1:oPkhp1MJrh7nUepCBc
golang.org/x/sys v0.0.0-20220722155257-8c9f86f7a55f/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/sys v0.5.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/sys v0.17.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA=
golang.org/x/sys v0.18.0 h1:DBdB3niSjOA/O0blCZBqDefyWNYveAYMNF1Wum0DYQ4=
golang.org/x/sys v0.18.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA=
golang.org/x/sys v0.19.0 h1:q5f1RH2jigJ1MoAWp2KTp3gm5zAGFUTarQZ5U386+4o=
golang.org/x/sys v0.19.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA=
golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo=
golang.org/x/term v0.0.0-20210927222741-03fcf44c2211/go.mod h1:jbD1KX2456YbFQfuXm/mYQcufACuNUgVhRMnK/tPxf8=
golang.org/x/term v0.5.0/go.mod h1:jMB1sMXY+tzblOD4FWmEbocvup2/aLOaQEp7JmGp78k=
golang.org/x/term v0.17.0/go.mod h1:lLRBjIVuehSbZlaOtGMbcMncT+aqLLLmKrsjNrUguwk=
golang.org/x/term v0.18.0 h1:FcHjZXDMxI8mM3nwhX9HlKop4C0YQvCVCdwYl2wOtE8=
golang.org/x/term v0.18.0/go.mod h1:ILwASektA3OnRv7amZ1xhE/KTR+u50pbXfZ03+6Nx58=
golang.org/x/term v0.19.0 h1:+ThwsDv+tYfnJFhF4L8jITxu1tdTWRTZpdsWgEgjL6Q=
golang.org/x/term v0.19.0/go.mod h1:2CuTdWZ7KHSQwUzKva0cbMg6q2DMI3Mmxp+gKJbskEk=
golang.org/x/text v0.0.0-20170915032832-14c0d48ead0c/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
golang.org/x/text v0.3.1-0.20180807135948-17ff2d5776d2/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
Expand Down
3 changes: 2 additions & 1 deletion pkg/gui/background.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -87,9 +87,10 @@ func (self *BackgroundRoutineMgr) goEvery(interval time.Duration, stop chan stru
if self.pauseBackgroundRefreshes {
continue
}
self.gui.c.OnWorker(func(gocui.Task) {
self.gui.c.OnWorker(func(gocui.Task) error {
_ = function()
done <- struct{}{}
return nil
})
// waiting so that we don't bunch up refreshes if the refresh takes longer than the interval
<-done
Expand Down
4 changes: 3 additions & 1 deletion pkg/gui/context/menu_context.go
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package context

import (
"errors"

"github.com/jesseduffield/lazygit/pkg/gui/keybindings"
"github.com/jesseduffield/lazygit/pkg/gui/style"
"github.com/jesseduffield/lazygit/pkg/gui/types"
Expand Down Expand Up @@ -148,7 +150,7 @@ func (self *MenuContext) GetKeybindings(opts types.KeybindingsOpts) []*types.Bin
func (self *MenuContext) OnMenuPress(selectedItem *types.MenuItem) error {
if selectedItem != nil && selectedItem.DisabledReason != nil {
if selectedItem.DisabledReason.ShowErrorInPanel {
return self.c.ErrorMsg(selectedItem.DisabledReason.Text)
return errors.New(selectedItem.DisabledReason.Text)
}

self.c.ErrorToast(self.c.Tr.DisabledMenuItemPrefix + selectedItem.DisabledReason.Text)
Expand Down
29 changes: 15 additions & 14 deletions pkg/gui/controllers/basic_commits_controller.go
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package controllers

import (
"errors"
"fmt"

"github.com/jesseduffield/lazygit/pkg/commands/git_commands"
Expand Down Expand Up @@ -172,7 +173,7 @@ func (self *BasicCommitsController) copyCommitAttribute(commit *models.Commit) e
func (self *BasicCommitsController) copyCommitHashToClipboard(commit *models.Commit) error {
self.c.LogAction(self.c.Tr.Actions.CopyCommitHashToClipboard)
if err := self.c.OS().CopyToClipboard(commit.Hash); err != nil {
return self.c.Error(err)
return err
}

self.c.Toast(fmt.Sprintf("'%s' %s", commit.Hash, self.c.Tr.CopiedToClipboard))
Expand All @@ -182,12 +183,12 @@ func (self *BasicCommitsController) copyCommitHashToClipboard(commit *models.Com
func (self *BasicCommitsController) copyCommitURLToClipboard(commit *models.Commit) error {
url, err := self.c.Helpers().Host.GetCommitURL(commit.Hash)
if err != nil {
return self.c.Error(err)
return err
}

self.c.LogAction(self.c.Tr.Actions.CopyCommitURLToClipboard)
if err := self.c.OS().CopyToClipboard(url); err != nil {
return self.c.Error(err)
return err
}

self.c.Toast(self.c.Tr.CommitURLCopiedToClipboard)
Expand All @@ -197,12 +198,12 @@ func (self *BasicCommitsController) copyCommitURLToClipboard(commit *models.Comm
func (self *BasicCommitsController) copyCommitDiffToClipboard(commit *models.Commit) error {
diff, err := self.c.Git().Commit.GetCommitDiff(commit.Hash)
if err != nil {
return self.c.Error(err)
return err
}

self.c.LogAction(self.c.Tr.Actions.CopyCommitDiffToClipboard)
if err := self.c.OS().CopyToClipboard(diff); err != nil {
return self.c.Error(err)
return err
}

self.c.Toast(self.c.Tr.CommitDiffCopiedToClipboard)
Expand All @@ -212,14 +213,14 @@ func (self *BasicCommitsController) copyCommitDiffToClipboard(commit *models.Com
func (self *BasicCommitsController) copyAuthorToClipboard(commit *models.Commit) error {
author, err := self.c.Git().Commit.GetCommitAuthor(commit.Hash)
if err != nil {
return self.c.Error(err)
return err
}

formattedAuthor := fmt.Sprintf("%s <%s>", author.Name, author.Email)

self.c.LogAction(self.c.Tr.Actions.CopyCommitAuthorToClipboard)
if err := self.c.OS().CopyToClipboard(formattedAuthor); err != nil {
return self.c.Error(err)
return err
}

self.c.Toast(self.c.Tr.CommitAuthorCopiedToClipboard)
Expand All @@ -229,12 +230,12 @@ func (self *BasicCommitsController) copyAuthorToClipboard(commit *models.Commit)
func (self *BasicCommitsController) copyCommitMessageToClipboard(commit *models.Commit) error {
message, err := self.c.Git().Commit.GetCommitMessage(commit.Hash)
if err != nil {
return self.c.Error(err)
return err
}

self.c.LogAction(self.c.Tr.Actions.CopyCommitMessageToClipboard)
if err := self.c.OS().CopyToClipboard(message); err != nil {
return self.c.Error(err)
return err
}

self.c.Toast(self.c.Tr.CommitMessageCopiedToClipboard)
Expand All @@ -244,12 +245,12 @@ func (self *BasicCommitsController) copyCommitMessageToClipboard(commit *models.
func (self *BasicCommitsController) copyCommitSubjectToClipboard(commit *models.Commit) error {
message, err := self.c.Git().Commit.GetCommitSubject(commit.Hash)
if err != nil {
return self.c.Error(err)
return err
}

self.c.LogAction(self.c.Tr.Actions.CopyCommitSubjectToClipboard)
if err := self.c.OS().CopyToClipboard(message); err != nil {
return self.c.Error(err)
return err
}

self.c.Toast(self.c.Tr.CommitSubjectCopiedToClipboard)
Expand All @@ -259,12 +260,12 @@ func (self *BasicCommitsController) copyCommitSubjectToClipboard(commit *models.
func (self *BasicCommitsController) openInBrowser(commit *models.Commit) error {
url, err := self.c.Helpers().Host.GetCommitURL(commit.Hash)
if err != nil {
return self.c.Error(err)
return err
}

self.c.LogAction(self.c.Tr.Actions.OpenCommitInBrowser)
if err := self.c.OS().OpenLink(url); err != nil {
return self.c.Error(err)
return err
}

return nil
Expand Down Expand Up @@ -314,7 +315,7 @@ func (self *BasicCommitsController) handleOldCherryPickKey() error {
"paste": keybindings.Label(self.c.UserConfig.Keybinding.Commits.PasteCommits),
})

return self.c.ErrorMsg(msg)
return errors.New(msg)
}

func (self *BasicCommitsController) openDiffTool(commit *models.Commit) error {
Expand Down
31 changes: 16 additions & 15 deletions pkg/gui/controllers/bisect_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ func (self *BisectController) openMidBisectMenu(info *git_commands.BisectInfo, c
OnPress: func() error {
self.c.LogAction(self.c.Tr.Actions.BisectMark)
if err := self.c.Git().Bisect.Mark(hashToMark, info.NewTerm()); err != nil {
return self.c.Error(err)
return err
}

return self.afterMark(selectCurrentAfter, waitToReselect)
Expand All @@ -109,7 +109,7 @@ func (self *BisectController) openMidBisectMenu(info *git_commands.BisectInfo, c
OnPress: func() error {
self.c.LogAction(self.c.Tr.Actions.BisectMark)
if err := self.c.Git().Bisect.Mark(hashToMark, info.OldTerm()); err != nil {
return self.c.Error(err)
return err
}

return self.afterMark(selectCurrentAfter, waitToReselect)
Expand All @@ -122,7 +122,7 @@ func (self *BisectController) openMidBisectMenu(info *git_commands.BisectInfo, c
OnPress: func() error {
self.c.LogAction(self.c.Tr.Actions.BisectSkip)
if err := self.c.Git().Bisect.Skip(hashToMark); err != nil {
return self.c.Error(err)
return err
}

return self.afterMark(selectCurrentAfter, waitToReselect)
Expand All @@ -137,7 +137,7 @@ func (self *BisectController) openMidBisectMenu(info *git_commands.BisectInfo, c
OnPress: func() error {
self.c.LogAction(self.c.Tr.Actions.BisectSkip)
if err := self.c.Git().Bisect.Skip(commit.Hash); err != nil {
return self.c.Error(err)
return err
}

return self.afterMark(selectCurrentAfter, waitToReselect)
Expand Down Expand Up @@ -169,11 +169,11 @@ func (self *BisectController) openStartBisectMenu(info *git_commands.BisectInfo,
OnPress: func() error {
self.c.LogAction(self.c.Tr.Actions.StartBisect)
if err := self.c.Git().Bisect.Start(); err != nil {
return self.c.Error(err)
return err
}

if err := self.c.Git().Bisect.Mark(commit.Hash, info.NewTerm()); err != nil {
return self.c.Error(err)
return err
}

return self.c.Helpers().Bisect.PostBisectCommandRefresh()
Expand All @@ -186,11 +186,11 @@ func (self *BisectController) openStartBisectMenu(info *git_commands.BisectInfo,
OnPress: func() error {
self.c.LogAction(self.c.Tr.Actions.StartBisect)
if err := self.c.Git().Bisect.Start(); err != nil {
return self.c.Error(err)
return err
}

if err := self.c.Git().Bisect.Mark(commit.Hash, info.OldTerm()); err != nil {
return self.c.Error(err)
return err
}

return self.c.Helpers().Bisect.PostBisectCommandRefresh()
Expand All @@ -209,7 +209,7 @@ func (self *BisectController) openStartBisectMenu(info *git_commands.BisectInfo,
HandleConfirm: func(newTerm string) error {
self.c.LogAction(self.c.Tr.Actions.StartBisect)
if err := self.c.Git().Bisect.StartWithTerms(oldTerm, newTerm); err != nil {
return self.c.Error(err)
return err
}

return self.c.Helpers().Bisect.PostBisectCommandRefresh()
Expand All @@ -232,7 +232,7 @@ func (self *BisectController) showBisectCompleteMessage(candidateHashes []string

formattedCommits, err := self.c.Git().Commit.GetCommitsOneline(candidateHashes)
if err != nil {
return self.c.Error(err)
return err
}

return self.c.Confirm(types.ConfirmOpts{
Expand All @@ -241,7 +241,7 @@ func (self *BisectController) showBisectCompleteMessage(candidateHashes []string
HandleConfirm: func() error {
self.c.LogAction(self.c.Tr.Actions.ResetBisect)
if err := self.c.Git().Bisect.Reset(); err != nil {
return self.c.Error(err)
return err
}

return self.c.Helpers().Bisect.PostBisectCommandRefresh()
Expand All @@ -252,11 +252,11 @@ func (self *BisectController) showBisectCompleteMessage(candidateHashes []string
func (self *BisectController) afterMark(selectCurrent bool, waitToReselect bool) error {
done, candidateHashes, err := self.c.Git().Bisect.IsDone()
if err != nil {
return self.c.Error(err)
return err
}

if err := self.afterBisectMarkRefresh(selectCurrent, waitToReselect); err != nil {
return self.c.Error(err)
return err
}

if done {
Expand All @@ -267,16 +267,17 @@ func (self *BisectController) afterMark(selectCurrent bool, waitToReselect bool)
}

func (self *BisectController) afterBisectMarkRefresh(selectCurrent bool, waitToReselect bool) error {
selectFn := func() {
selectFn := func() error {
if selectCurrent {
self.selectCurrentBisectCommit()
}
return nil
}

if waitToReselect {
return self.c.Refresh(types.RefreshOptions{Mode: types.SYNC, Scope: []types.RefreshableView{}, Then: selectFn})
} else {
selectFn()
_ = selectFn()

return self.c.Helpers().Bisect.PostBisectCommandRefresh()
}
Expand Down
Loading

0 comments on commit 34f8f72

Please sign in to comment.