Show Toast instead of error panel when invoking a disabled command (#3180)

- **PR Description**

Addresses #3116.

I'm not 100% sure I like the behavior, but I put it out there so that
others can test it and form an opinion. It not only affects keybindings,
but also invoking menu items (either with enter or with their key
binding): the menu now stays open in that case, which I think is
actually better.

There's a horrible hack for keeping the integration tests working, I
don't have a good idea how to fix that for real. Suggestions welcome.

Author: jesseduffield

pkg/app/app.go

@@ -23,6 +23,7 @@ import (
 	"github.com/jesseduffield/lazygit/pkg/env"
 	"github.com/jesseduffield/lazygit/pkg/gui"
 	"github.com/jesseduffield/lazygit/pkg/i18n"
+	integrationTypes "github.com/jesseduffield/lazygit/pkg/integration/types"
 	"github.com/jesseduffield/lazygit/pkg/logs"
 	"github.com/jesseduffield/lazygit/pkg/updates"
 )
@@ -42,7 +43,7 @@ func Run(
 	common *common.Common,
 	startArgs appTypes.StartArgs,
 ) {
-	app, err := NewApp(config, common)
+	app, err := NewApp(config, startArgs.IntegrationTest, common)
 
 	if err == nil {
 		err = app.Run(startArgs)
@@ -94,7 +95,7 @@ func newLogger(cfg config.AppConfigurer) *logrus.Entry {
 }
 
 // NewApp bootstrap a new application
-func NewApp(config config.AppConfigurer, common *common.Common) (*App, error) {
+func NewApp(config config.AppConfigurer, test integrationTypes.IntegrationTest, common *common.Common) (*App, error) {
 	app := &App{
 		closers: []io.Closer{},
 		Config:  config,
@@ -128,7 +129,7 @@ func NewApp(config config.AppConfigurer, common *common.Common) (*App, error) {
 		showRecentRepos = true
 	}
 
-	app.Gui, err = gui.NewGui(common, config, gitVersion, updater, showRecentRepos, dirName)
+	app.Gui, err = gui.NewGui(common, config, gitVersion, updater, showRecentRepos, dirName, test)
 	if err != nil {
 		return app, err
 	}

pkg/cheatsheet/generate.go

@@ -61,7 +61,7 @@ func generateAtDir(cheatsheetDir string) {
 		if err != nil {
 			log.Fatal(err)
 		}
-		mApp, _ := app.NewApp(mConfig, common)
+		mApp, _ := app.NewApp(mConfig, nil, common)
 		path := cheatsheetDir + "/Keybindings_" + lang + ".md"
 		file, err := os.Create(path)
 		if err != nil {

pkg/gui/context/menu_context.go

@@ -90,7 +90,7 @@ func (self *MenuViewModel) GetDisplayStrings(_ int, _ int) [][]string {
 
 	return lo.Map(menuItems, func(item *types.MenuItem, _ int) []string {
 		displayStrings := item.LabelColumns
-		if item.DisabledReason != "" {
+		if item.DisabledReason != nil {
 			displayStrings[0] = style.FgDefault.SetStrikethrough().Sprint(displayStrings[0])
 		}
 
@@ -172,8 +172,13 @@ func (self *MenuContext) GetKeybindings(opts types.KeybindingsOpts) []*types.Bin
 }
 
 func (self *MenuContext) OnMenuPress(selectedItem *types.MenuItem) error {
-	if selectedItem != nil && selectedItem.DisabledReason != "" {
-		return self.c.ErrorMsg(selectedItem.DisabledReason)
+	if selectedItem != nil && selectedItem.DisabledReason != nil {
+		if selectedItem.DisabledReason.ShowErrorInPanel {
+			return self.c.ErrorMsg(selectedItem.DisabledReason.Text)
+		}
+
+		self.c.ErrorToast(self.c.Tr.DisabledMenuItemPrefix + selectedItem.DisabledReason.Text)
+		return nil
 	}
 
 	if err := self.c.PopContext(); err != nil {

pkg/gui/controllers/branches_controller.go

@@ -264,13 +264,13 @@ func (self *BranchesController) viewUpstreamOptions(selectedBranch *models.Branc
 	}
 
 	if !selectedBranch.IsTrackingRemote() {
-		unsetUpstreamItem.DisabledReason = self.c.Tr.UpstreamNotSetError
+		unsetUpstreamItem.DisabledReason = &types.DisabledReason{Text: self.c.Tr.UpstreamNotSetError}
 	}
 
 	if !selectedBranch.RemoteBranchStoredLocally() {
-		viewDivergenceItem.DisabledReason = self.c.Tr.UpstreamNotSetError
-		upstreamResetItem.DisabledReason = self.c.Tr.UpstreamNotSetError
-		upstreamRebaseItem.DisabledReason = self.c.Tr.UpstreamNotSetError
+		viewDivergenceItem.DisabledReason = &types.DisabledReason{Text: self.c.Tr.UpstreamNotSetError}
+		upstreamResetItem.DisabledReason = &types.DisabledReason{Text: self.c.Tr.UpstreamNotSetError}
+		upstreamRebaseItem.DisabledReason = &types.DisabledReason{Text: self.c.Tr.UpstreamNotSetError}
 	}
 
 	options := []*types.MenuItem{
@@ -309,16 +309,16 @@ func (self *BranchesController) press(selectedBranch *models.Branch) error {
 	return self.c.Helpers().Refs.CheckoutRef(selectedBranch.Name, types.CheckoutRefOptions{})
 }
 
-func (self *BranchesController) getDisabledReasonForPress() string {
+func (self *BranchesController) getDisabledReasonForPress() *types.DisabledReason {
 	currentBranch := self.c.Helpers().Refs.GetCheckedOutRef()
 	if currentBranch != nil {
 		op := self.c.State().GetItemOperation(currentBranch)
 		if op == types.ItemOperationFastForwarding || op == types.ItemOperationPulling {
-			return self.c.Tr.CantCheckoutBranchWhilePulling
+			return &types.DisabledReason{Text: self.c.Tr.CantCheckoutBranchWhilePulling}
 		}
 	}
 
-	return ""
+	return nil
 }
 
 func (self *BranchesController) worktreeForBranch(branch *models.Branch) (*models.Worktree, bool) {
@@ -525,7 +525,7 @@ func (self *BranchesController) delete(branch *models.Branch) error {
 		},
 	}
 	if checkedOutBranch.Name == branch.Name {
-		localDeleteItem.DisabledReason = self.c.Tr.CantDeleteCheckOutBranch
+		localDeleteItem.DisabledReason = &types.DisabledReason{Text: self.c.Tr.CantDeleteCheckOutBranch}
 	}
 
 	remoteDeleteItem := &types.MenuItem{
@@ -536,7 +536,7 @@ func (self *BranchesController) delete(branch *models.Branch) error {
 		},
 	}
 	if !branch.IsTrackingRemote() || branch.UpstreamGone {
-		remoteDeleteItem.DisabledReason = self.c.Tr.UpstreamNotSetError
+		remoteDeleteItem.DisabledReason = &types.DisabledReason{Text: self.c.Tr.UpstreamNotSetError}
 	}
 
 	menuTitle := utils.ResolvePlaceholderString(
@@ -562,14 +562,14 @@ func (self *BranchesController) rebase() error {
 	return self.c.Helpers().MergeAndRebase.RebaseOntoRef(selectedBranchName)
 }
 
-func (self *BranchesController) getDisabledReasonForRebase() string {
+func (self *BranchesController) getDisabledReasonForRebase() *types.DisabledReason {
 	selectedBranchName := self.context().GetSelected().Name
 	checkedOutBranch := self.c.Helpers().Refs.GetCheckedOutRef().Name
 	if selectedBranchName == checkedOutBranch {
-		return self.c.Tr.CantRebaseOntoSelf
+		return &types.DisabledReason{Text: self.c.Tr.CantRebaseOntoSelf}
 	}
 
-	return ""
+	return nil
 }
 
 func (self *BranchesController) fastForward(branch *models.Branch) error {

pkg/gui/controllers/files_controller.go

@@ -848,15 +848,15 @@ func (self *FilesController) openCopyMenu() error {
 	}
 
 	if node == nil {
-		copyNameItem.DisabledReason = self.c.Tr.NoContentToCopyError
-		copyPathItem.DisabledReason = self.c.Tr.NoContentToCopyError
-		copyFileDiffItem.DisabledReason = self.c.Tr.NoContentToCopyError
+		copyNameItem.DisabledReason = &types.DisabledReason{Text: self.c.Tr.NoContentToCopyError}
+		copyPathItem.DisabledReason = &types.DisabledReason{Text: self.c.Tr.NoContentToCopyError}
+		copyFileDiffItem.DisabledReason = &types.DisabledReason{Text: self.c.Tr.NoContentToCopyError}
 	}
 	if node != nil && !node.GetHasStagedOrTrackedChanges() {
-		copyFileDiffItem.DisabledReason = self.c.Tr.NoContentToCopyError
+		copyFileDiffItem.DisabledReason = &types.DisabledReason{Text: self.c.Tr.NoContentToCopyError}
 	}
 	if !self.anyStagedOrTrackedFile() {
-		copyAllDiff.DisabledReason = self.c.Tr.NoContentToCopyError
+		copyAllDiff.DisabledReason = &types.DisabledReason{Text: self.c.Tr.NoContentToCopyError}
 	}
 
 	return self.c.Menu(types.CreateMenuOptions{

pkg/gui/controllers/helpers/app_status_helper.go

@@ -5,6 +5,7 @@ import (
 
 	"github.com/jesseduffield/gocui"
 	"github.com/jesseduffield/lazygit/pkg/gui/status"
+	"github.com/jesseduffield/lazygit/pkg/gui/types"
 	"github.com/jesseduffield/lazygit/pkg/utils"
 )
 
@@ -23,15 +24,15 @@ func NewAppStatusHelper(c *HelperCommon, statusMgr func() *status.StatusManager,
 	}
 }
 
-func (self *AppStatusHelper) Toast(message string) {
+func (self *AppStatusHelper) Toast(message string, kind types.ToastKind) {
 	if self.c.RunningIntegrationTest() {
 		// Don't bother showing toasts in integration tests. You can't check for
 		// them anyway, and they would only slow down the test unnecessarily by
 		// two seconds.
 		return
 	}
 
-	self.statusMgr().AddToastStatus(message)
+	self.statusMgr().AddToastStatus(message, kind)
 
 	self.renderAppStatus()
 }
@@ -87,15 +88,17 @@ func (self *AppStatusHelper) HasStatus() bool {
 }
 
 func (self *AppStatusHelper) GetStatusString() string {
-	return self.statusMgr().GetStatusString()
+	appStatus, _ := self.statusMgr().GetStatusString()
+	return appStatus
 }
 
 func (self *AppStatusHelper) renderAppStatus() {
 	self.c.OnWorker(func(_ gocui.Task) {
 		ticker := time.NewTicker(time.Millisecond * utils.LoaderAnimationInterval)
 		defer ticker.Stop()
 		for range ticker.C {
-			appStatus := self.statusMgr().GetStatusString()
+			appStatus, color := self.statusMgr().GetStatusString()
+			self.c.Views().AppStatus.FgColor = color
 			self.c.OnUIThread(func() error {
 				self.c.SetViewContent(self.c.Views().AppStatus, appStatus)
 				return nil
@@ -127,7 +130,8 @@ func (self *AppStatusHelper) renderAppStatusSync(stop chan struct{}) {
 		for {
 			select {
 			case <-ticker.C:
-				appStatus := self.statusMgr().GetStatusString()
+				appStatus, color := self.statusMgr().GetStatusString()
+				self.c.Views().AppStatus.FgColor = color
 				self.c.SetViewContent(self.c.Views().AppStatus, appStatus)
 				// Redraw all views of the bottom line:
 				bottomLineViews := []*gocui.View{

pkg/gui/controllers/helpers/confirmation_helper.go

@@ -378,11 +378,11 @@ func (self *ConfirmationHelper) IsPopupPanelFocused() bool {
 
 func (self *ConfirmationHelper) TooltipForMenuItem(menuItem *types.MenuItem) string {
 	tooltip := menuItem.Tooltip
-	if menuItem.DisabledReason != "" {
+	if menuItem.DisabledReason != nil {
 		if tooltip != "" {
 			tooltip += "\n\n"
 		}
-		tooltip += style.FgRed.Sprintf(self.c.Tr.DisabledMenuItemPrefix) + menuItem.DisabledReason
+		tooltip += style.FgRed.Sprintf(self.c.Tr.DisabledMenuItemPrefix) + menuItem.DisabledReason.Text
 	}
 	return tooltip
 }