From ee173ff7c9f0a7c4279982c2361c12e0d1897504 Mon Sep 17 00:00:00 2001 From: molejnik88 Date: Sat, 20 Jan 2024 13:55:25 +0000 Subject: [PATCH] Clear cherry-picked commits after pasting It can be tedious after each cherry-pick opearation to clear the selection by pressing escape in order for lazygit to stop displaying info about copied commits. Also, it seems to be a rare case to cherry-pick commits to more than one destination. The simplest solution to address this issue is to clear the selection upon paste. The only exception is a merge conflict. Initially, I wanted to clear selected commits in this scenario too. During a discussion we found out that it may be convenient to have the copied commits still around. Aborting the rebase and pasting the commits in the middle of a branch can be a valid use case. --- .../controllers/helpers/cherry_pick_helper.go | 25 +++++++++++++++++-- .../tests/cherry_pick/cherry_pick.go | 17 ++++++------- .../cherry_pick/cherry_pick_conflicts.go | 4 +++ .../cherry_pick/cherry_pick_during_rebase.go | 3 +++ .../tests/cherry_pick/cherry_pick_range.go | 13 +++------- 5 files changed, 42 insertions(+), 20 deletions(-) diff --git a/pkg/gui/controllers/helpers/cherry_pick_helper.go b/pkg/gui/controllers/helpers/cherry_pick_helper.go index 61a37220b..137d0626d 100644 --- a/pkg/gui/controllers/helpers/cherry_pick_helper.go +++ b/pkg/gui/controllers/helpers/cherry_pick_helper.go @@ -90,15 +90,36 @@ func (self *CherryPickHelper) Paste() error { if err := self.c.Git().Rebase.CherryPickCommitsDuringRebase(self.getData().CherryPickedCommits); err != nil { return err } - return self.c.Refresh(types.RefreshOptions{ + err = self.c.Refresh(types.RefreshOptions{ Mode: types.SYNC, Scope: []types.RefreshableView{types.REBASE_COMMITS}, }) + if err != nil { + return err + } + + return self.Reset() } return self.c.WithWaitingStatus(self.c.Tr.CherryPickingStatus, func(gocui.Task) error { self.c.LogAction(self.c.Tr.Actions.CherryPick) err := self.c.Git().Rebase.CherryPickCommits(self.getData().CherryPickedCommits) - return self.rebaseHelper.CheckMergeOrRebase(err) + err = self.rebaseHelper.CheckMergeOrRebase(err) + if err != nil { + return err + } + + // If we're in an interactive rebase at this point, it must + // be because there were conflicts. Don't clear the copied + // commits in this case, since we might want to abort and + // try pasting them again. + isInRebase, err = self.c.Git().Status.IsInInteractiveRebase() + if err != nil { + return err + } + if !isInRebase { + return self.Reset() + } + return nil }) }, }) diff --git a/pkg/integration/tests/cherry_pick/cherry_pick.go b/pkg/integration/tests/cherry_pick/cherry_pick.go index f0778a05a..c49e5cf38 100644 --- a/pkg/integration/tests/cherry_pick/cherry_pick.go +++ b/pkg/integration/tests/cherry_pick/cherry_pick.go @@ -59,26 +59,25 @@ var CherryPick = NewIntegrationTest(NewIntegrationTestArgs{ Contains("base"), ). Press(keys.Commits.PasteCommits). + Tap(func() { + // cherry-picked commits will be deleted after confirmation + t.Views().Information().Content(Contains("2 commits copied")) + }). Tap(func() { t.ExpectPopup().Alert(). Title(Equals("Cherry-pick")). Content(Contains("Are you sure you want to cherry-pick the copied commits onto this branch?")). Confirm() }). + Tap(func() { + t.Views().Information().Content(DoesNotContain("commits copied")) + }). Lines( Contains("four"), Contains("three"), Contains("two"), Contains("one"), Contains("base"), - ). - Tap(func() { - // we need to manually exit out of cherry pick mode - t.Views().Information().Content(Contains("2 commits copied")) - }). - PressEscape(). - Tap(func() { - t.Views().Information().Content(DoesNotContain("commits copied")) - }) + ) }, }) diff --git a/pkg/integration/tests/cherry_pick/cherry_pick_conflicts.go b/pkg/integration/tests/cherry_pick/cherry_pick_conflicts.go index b2ec0b24f..b5b4e1fd9 100644 --- a/pkg/integration/tests/cherry_pick/cherry_pick_conflicts.go +++ b/pkg/integration/tests/cherry_pick/cherry_pick_conflicts.go @@ -54,6 +54,10 @@ var CherryPickConflicts = NewIntegrationTest(NewIntegrationTestArgs{ t.Common().AcknowledgeConflicts() + // cherry pick selection is not cleared when there are conflicts, so that the user + // is able to abort and try again without having to re-copy the commits + t.Views().Information().Content(Contains("2 commits copied")) + t.Views().Files(). IsFocused(). SelectedLine(Contains("file")). diff --git a/pkg/integration/tests/cherry_pick/cherry_pick_during_rebase.go b/pkg/integration/tests/cherry_pick/cherry_pick_during_rebase.go index 4cf32bad3..caeb7bfe0 100644 --- a/pkg/integration/tests/cherry_pick/cherry_pick_during_rebase.go +++ b/pkg/integration/tests/cherry_pick/cherry_pick_during_rebase.go @@ -68,6 +68,9 @@ var CherryPickDuringRebase = NewIntegrationTest(NewIntegrationTestArgs{ Content(Contains("Are you sure you want to cherry-pick the copied commits onto this branch?")). Confirm() }). + Tap(func() { + t.Views().Information().Content(DoesNotContain("commit copied")) + }). Lines( Contains("pick CI two"), Contains("pick CI three"), diff --git a/pkg/integration/tests/cherry_pick/cherry_pick_range.go b/pkg/integration/tests/cherry_pick/cherry_pick_range.go index 99b29618f..e68b9bd46 100644 --- a/pkg/integration/tests/cherry_pick/cherry_pick_range.go +++ b/pkg/integration/tests/cherry_pick/cherry_pick_range.go @@ -66,20 +66,15 @@ var CherryPickRange = NewIntegrationTest(NewIntegrationTestArgs{ Content(Contains("Are you sure you want to cherry-pick the copied commits onto this branch?")). Confirm() }). + Tap(func() { + t.Views().Information().Content(DoesNotContain("commits copied")) + }). Lines( Contains("four"), Contains("three"), Contains("two"), Contains("one"), Contains("base"), - ). - Tap(func() { - // we need to manually exit out of cherry pick mode - t.Views().Information().Content(Contains("2 commits copied")) - }). - PressEscape(). - Tap(func() { - t.Views().Information().Content(DoesNotContain("commits copied")) - }) + ) }, })