From 114ad52ff61346bb4c4eaa64f157197ebfb998a5 Mon Sep 17 00:00:00 2001 From: Jesse Duffield Date: Sat, 13 May 2023 20:43:26 +1000 Subject: [PATCH 1/4] Rename CmdLog -> GuiLog We want to log both actions and commands for the sake of integration tests --- pkg/gui/command_log_panel.go | 3 ++- pkg/gui/gui.go | 6 +++--- pkg/gui/gui_driver.go | 2 +- 3 files changed, 6 insertions(+), 5 deletions(-) diff --git a/pkg/gui/command_log_panel.go b/pkg/gui/command_log_panel.go index 258a9cac1..c0aa83bec 100644 --- a/pkg/gui/command_log_panel.go +++ b/pkg/gui/command_log_panel.go @@ -30,6 +30,7 @@ func (gui *Gui) LogAction(action string) { gui.Views.Extras.Autoscroll = true + gui.GuiLog = append(gui.GuiLog, action) fmt.Fprint(gui.Views.Extras, "\n"+style.FgYellow.Sprint(action)) } @@ -46,7 +47,7 @@ func (gui *Gui) LogCommand(cmdStr string, commandLine bool) { // we style it differently to communicate that textStyle = style.FgMagenta } - gui.CmdLog = append(gui.CmdLog, cmdStr) + gui.GuiLog = append(gui.GuiLog, cmdStr) indentedCmdStr := " " + strings.Replace(cmdStr, "\n", "\n ", -1) fmt.Fprint(gui.Views.Extras, "\n"+textStyle.Sprint(indentedCmdStr)) } diff --git a/pkg/gui/gui.go b/pkg/gui/gui.go index 6b42da4fc..b429bda27 100644 --- a/pkg/gui/gui.go +++ b/pkg/gui/gui.go @@ -93,8 +93,8 @@ type Gui struct { Views types.Views - // Log of the commands that get run, to be displayed to the user. - CmdLog []string + // Log of the commands/actions logged in the Command Log panel. + GuiLog []string // the extras window contains things like the command log ShowExtrasWindow bool @@ -440,7 +440,7 @@ func NewGui( showRecentRepos: showRecentRepos, RepoPathStack: &utils.StringStack{}, RepoStateMap: map[Repo]*GuiRepoState{}, - CmdLog: []string{}, + GuiLog: []string{}, // originally we could only hide the command log permanently via the config // but now we do it via state. So we need to still support the config for the diff --git a/pkg/gui/gui_driver.go b/pkg/gui/gui_driver.go index c409ebdb9..a90578b65 100644 --- a/pkg/gui/gui_driver.go +++ b/pkg/gui/gui_driver.go @@ -64,7 +64,7 @@ func (self *GuiDriver) Fail(message string) { "%s\nFinal Lazygit state:\n%s\nUpon failure, focused view was '%s'.\nLog:\n%s", message, self.gui.g.Snapshot(), currentView.Name(), - strings.Join(self.gui.CmdLog, "\n"), + strings.Join(self.gui.GuiLog, "\n"), ) self.gui.g.Close() From 95926866290980855dbefd16b503cb106e1c6171 Mon Sep 17 00:00:00 2001 From: Jesse Duffield Date: Tue, 16 May 2023 20:11:24 +1000 Subject: [PATCH 2/4] Compare contexts with keys We don't want to compare contexts directly given they are interfaces and not pointers to structs --- pkg/gui/context.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/gui/context.go b/pkg/gui/context.go index d5a7f35a2..7d3b32a1d 100644 --- a/pkg/gui/context.go +++ b/pkg/gui/context.go @@ -95,7 +95,7 @@ func (self *ContextMgr) pushToContextStack(c types.Context) ([]types.Context, ty defer self.Unlock() if len(self.ContextStack) > 0 && - c == self.ContextStack[len(self.ContextStack)-1] { + c.GetKey() == self.ContextStack[len(self.ContextStack)-1].GetKey() { // Context being pushed is already on top of the stack: nothing to // deactivate or activate return contextsToDeactivate, nil From 00b03079d88464916528df720d8b85f08ce6a6a3 Mon Sep 17 00:00:00 2001 From: Jesse Duffield Date: Tue, 16 May 2023 20:24:17 +1000 Subject: [PATCH 3/4] Don't deactivate context that you're about to activate --- pkg/gui/context.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/pkg/gui/context.go b/pkg/gui/context.go index 7d3b32a1d..b55713f27 100644 --- a/pkg/gui/context.go +++ b/pkg/gui/context.go @@ -105,7 +105,9 @@ func (self *ContextMgr) pushToContextStack(c types.Context) ([]types.Context, ty self.ContextStack = append(self.ContextStack, c) } else if c.GetKind() == types.SIDE_CONTEXT { // if we are switching to a side context, remove all other contexts in the stack - contextsToDeactivate = self.ContextStack + contextsToDeactivate = lo.Filter(self.ContextStack, func(context types.Context, _ int) bool { + return context.GetKey() != c.GetKey() + }) self.ContextStack = []types.Context{c} } else if c.GetKind() == types.MAIN_CONTEXT { // if we're switching to a main context, remove all other main contexts in the stack From a82134f41c60622166ad56c2b2c7f83e1989be77 Mon Sep 17 00:00:00 2001 From: Jesse Duffield Date: Tue, 16 May 2023 20:45:43 +1000 Subject: [PATCH 4/4] Fix race condition Our refresh code may try to push a context. It does this in two places: 1) when all merge conflicts are resolved, we push a 'continue merge?' confirmation context 2) when all conflicts of a given file are resolved and we're in the merge conflicts context, we push the files context. Sometimes we push the confirmation context and then push the files context over it, so the user never sees the confirmation context. This commit fixes the race condition by adding a check to ensure that we're still in the merge conflicts panel before we try escaping from it --- pkg/gui/controllers/helpers/merge_conflicts_helper.go | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/pkg/gui/controllers/helpers/merge_conflicts_helper.go b/pkg/gui/controllers/helpers/merge_conflicts_helper.go index 3610e28c8..e6a56bfae 100644 --- a/pkg/gui/controllers/helpers/merge_conflicts_helper.go +++ b/pkg/gui/controllers/helpers/merge_conflicts_helper.go @@ -56,7 +56,15 @@ func (self *MergeConflictsHelper) EscapeMerge() error { // doing this in separate UI thread so that we're not still holding the lock by the time refresh the file self.c.OnUIThread(func() error { - return self.c.PushContext(self.c.Contexts().Files) + // There is a race condition here: refreshing the files scope can trigger the + // confirmation context to be pushed if all conflicts are resolved (prompting + // to continue the merge/rebase. In that case, we don't want to then push the + // files context over it. + // So long as both places call OnUIThread, we're fine. + if self.c.IsCurrentContext(self.c.Contexts().MergeConflicts) { + return self.c.PushContext(self.c.Contexts().Files) + } + return nil }) return nil }