From 6794149ec872764ba0ad2d03d998bcd6ebb95f96 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Thu, 27 Jul 2023 18:21:47 +0200 Subject: [PATCH] When bisecting, always mark the current commit as good/bad, not the selected For marking as good or bad, the current commit is pretty much always the one you want to mark, not the selected. It's different for skipping; sometimes you know already that a certain commit doesn't compile, for example, so you might navigate there and mark it as skipped. So in the case that the current commit is not the selected one, we now offer two separate menu entries for skipping, one for the current commit and one for the selected. --- pkg/gui/controllers/bisect_controller.go | 50 ++++++++++---- pkg/i18n/chinese.go | 2 +- pkg/i18n/english.go | 8 ++- pkg/i18n/japanese.go | 2 +- pkg/i18n/korean.go | 2 +- pkg/i18n/russian.go | 2 +- pkg/i18n/traditional_chinese.go | 2 +- pkg/integration/tests/bisect/skip.go | 88 ++++++++++++++++++++++++ pkg/integration/tests/test_list.go | 1 + 9 files changed, 135 insertions(+), 22 deletions(-) create mode 100644 pkg/integration/tests/bisect/skip.go diff --git a/pkg/gui/controllers/bisect_controller.go b/pkg/gui/controllers/bisect_controller.go index afa417bb1..7cb36ef26 100644 --- a/pkg/gui/controllers/bisect_controller.go +++ b/pkg/gui/controllers/bisect_controller.go @@ -8,6 +8,8 @@ import ( "github.com/jesseduffield/lazygit/pkg/commands/models" "github.com/jesseduffield/lazygit/pkg/gui/context" "github.com/jesseduffield/lazygit/pkg/gui/types" + "github.com/jesseduffield/lazygit/pkg/utils" + "github.com/samber/lo" ) type BisectController struct { @@ -65,12 +67,18 @@ func (self *BisectController) openMidBisectMenu(info *git_commands.BisectInfo, c // ref, because we'll be reloading our commits in that case. waitToReselect := selectCurrentAfter && !self.c.Git().Bisect.ReachableFromStart(info) + // If we have a current sha already, then we always want to use that one. If + // not, we're still picking the initial commits before we really start, so + // use the selected commit in that case. + shaToMark := lo.Ternary(info.GetCurrentSha() != "", info.GetCurrentSha(), commit.Sha) + shortShaToMark := utils.ShortSha(shaToMark) + menuItems := []*types.MenuItem{ { - Label: fmt.Sprintf(self.c.Tr.Bisect.Mark, commit.ShortSha(), info.NewTerm()), + Label: fmt.Sprintf(self.c.Tr.Bisect.Mark, shortShaToMark, info.NewTerm()), OnPress: func() error { self.c.LogAction(self.c.Tr.Actions.BisectMark) - if err := self.c.Git().Bisect.Mark(commit.Sha, info.NewTerm()); err != nil { + if err := self.c.Git().Bisect.Mark(shaToMark, info.NewTerm()); err != nil { return self.c.Error(err) } @@ -79,10 +87,10 @@ func (self *BisectController) openMidBisectMenu(info *git_commands.BisectInfo, c Key: 'b', }, { - Label: fmt.Sprintf(self.c.Tr.Bisect.Mark, commit.ShortSha(), info.OldTerm()), + Label: fmt.Sprintf(self.c.Tr.Bisect.Mark, shortShaToMark, info.OldTerm()), OnPress: func() error { self.c.LogAction(self.c.Tr.Actions.BisectMark) - if err := self.c.Git().Bisect.Mark(commit.Sha, info.OldTerm()); err != nil { + if err := self.c.Git().Bisect.Mark(shaToMark, info.OldTerm()); err != nil { return self.c.Error(err) } @@ -91,7 +99,21 @@ func (self *BisectController) openMidBisectMenu(info *git_commands.BisectInfo, c Key: 'g', }, { - Label: fmt.Sprintf(self.c.Tr.Bisect.Skip, commit.ShortSha()), + Label: fmt.Sprintf(self.c.Tr.Bisect.SkipCurrent, shortShaToMark), + OnPress: func() error { + self.c.LogAction(self.c.Tr.Actions.BisectSkip) + if err := self.c.Git().Bisect.Skip(shaToMark); err != nil { + return self.c.Error(err) + } + + return self.afterMark(selectCurrentAfter, waitToReselect) + }, + Key: 's', + }, + } + if info.GetCurrentSha() != "" && info.GetCurrentSha() != commit.Sha { + menuItems = append(menuItems, lo.ToPtr(types.MenuItem{ + Label: fmt.Sprintf(self.c.Tr.Bisect.SkipSelected, commit.ShortSha()), OnPress: func() error { self.c.LogAction(self.c.Tr.Actions.BisectSkip) if err := self.c.Git().Bisect.Skip(commit.Sha); err != nil { @@ -100,16 +122,16 @@ func (self *BisectController) openMidBisectMenu(info *git_commands.BisectInfo, c return self.afterMark(selectCurrentAfter, waitToReselect) }, - Key: 's', - }, - { - Label: self.c.Tr.Bisect.ResetOption, - OnPress: func() error { - return self.c.Helpers().Bisect.Reset() - }, - Key: 'r', - }, + Key: 'S', + })) } + menuItems = append(menuItems, lo.ToPtr(types.MenuItem{ + Label: self.c.Tr.Bisect.ResetOption, + OnPress: func() error { + return self.c.Helpers().Bisect.Reset() + }, + Key: 'r', + })) return self.c.Menu(types.CreateMenuOptions{ Title: self.c.Tr.Bisect.BisectMenuTitle, diff --git a/pkg/i18n/chinese.go b/pkg/i18n/chinese.go index f6171f68e..2a494c97e 100644 --- a/pkg/i18n/chinese.go +++ b/pkg/i18n/chinese.go @@ -544,7 +544,7 @@ func chineseTranslationSet() TranslationSet { Bisect: Bisect{ Mark: "将 %s 标记为 %s", MarkStart: "将 %s 标记为 %s (start bisect)", - Skip: "跳过 %s", + SkipCurrent: "跳过 %s", ResetTitle: "重置 'git bisect'", ResetPrompt: "您确定要重置 'git bisect' 吗？", ResetOption: "重置二分查找", diff --git a/pkg/i18n/english.go b/pkg/i18n/english.go index 2d0fb90e8..34f9e66e0 100644 --- a/pkg/i18n/english.go +++ b/pkg/i18n/english.go @@ -557,7 +557,8 @@ type Bisect struct { NewTermPrompt string BisectMenuTitle string Mark string - Skip string + SkipCurrent string + SkipSelected string CompleteTitle string CompletePrompt string CompletePromptIndeterminate string @@ -1347,9 +1348,10 @@ func EnglishTranslationSet() TranslationSet { BisectMark: "Bisect mark", }, Bisect: Bisect{ - Mark: "Mark %s as %s", + Mark: "Mark current commit (%s) as %s", MarkStart: "Mark %s as %s (start bisect)", - Skip: "Skip %s", + SkipCurrent: "Skip current commit (%s)", + SkipSelected: "Skip selected commit (%s)", ResetTitle: "Reset 'git bisect'", ResetPrompt: "Are you sure you want to reset 'git bisect'?", ResetOption: "Reset bisect", diff --git a/pkg/i18n/japanese.go b/pkg/i18n/japanese.go index 6f77aa26d..3cbc6e225 100644 --- a/pkg/i18n/japanese.go +++ b/pkg/i18n/japanese.go @@ -577,7 +577,7 @@ func japaneseTranslationSet() TranslationSet { Bisect: Bisect{ // Mark: "Mark %s as %s", // MarkStart: "Mark %s as %s (start bisect)", - Skip: "%s をスキップする", + SkipCurrent: "%s をスキップする", ResetTitle: "'git bisect' をリセット", ResetPrompt: "'git bisect' をリセットします。よろしいですか?", ResetOption: "Bisectをリセット", diff --git a/pkg/i18n/korean.go b/pkg/i18n/korean.go index 801a83474..f22e6cc47 100644 --- a/pkg/i18n/korean.go +++ b/pkg/i18n/korean.go @@ -576,7 +576,7 @@ func koreanTranslationSet() TranslationSet { Bisect: Bisect{ Mark: "Mark %s as %s", MarkStart: "Mark %s as %s (start bisect)", - Skip: "%s 를 스킵", + SkipCurrent: "%s 를 스킵", ResetTitle: "'git bisect' 를 리셋", ResetPrompt: "정말로 'git bisect' 를 리셋하시겠습니까?", ResetOption: "Bisect를 리셋", diff --git a/pkg/i18n/russian.go b/pkg/i18n/russian.go index fe4785667..ae21430e6 100644 --- a/pkg/i18n/russian.go +++ b/pkg/i18n/russian.go @@ -671,7 +671,7 @@ func RussianTranslationSet() TranslationSet { Bisect: Bisect{ Mark: "Отметить %s как %s", MarkStart: "Отметить %s как %s (начать бинарный поиск)", - Skip: "Пропустить %s", + SkipCurrent: "Пропустить %s", ResetTitle: "Сбросить 'git bisect'", ResetPrompt: "Вы уверены, что хотите сбросить 'git bisect'?", ResetOption: "Сбросить бинарный поиск", diff --git a/pkg/i18n/traditional_chinese.go b/pkg/i18n/traditional_chinese.go index ebdda62bf..3ccac945a 100644 --- a/pkg/i18n/traditional_chinese.go +++ b/pkg/i18n/traditional_chinese.go @@ -698,7 +698,7 @@ func traditionalChineseTranslationSet() TranslationSet { Bisect: Bisect{ Mark: "將 %s 標記為 %s", MarkStart: "將 %s 標記為 %s（開始二分查找）", - Skip: "跳過 %s", + SkipCurrent: "跳過 %s", ResetTitle: "重設 'git bisect'", ResetPrompt: "你確定要重設 'git bisect' 嗎？", ResetOption: "重設二分查找", diff --git a/pkg/integration/tests/bisect/skip.go b/pkg/integration/tests/bisect/skip.go new file mode 100644 index 000000000..998e82361 --- /dev/null +++ b/pkg/integration/tests/bisect/skip.go @@ -0,0 +1,88 @@ +package bisect + +import ( + "github.com/jesseduffield/lazygit/pkg/config" + . "github.com/jesseduffield/lazygit/pkg/integration/components" +) + +var Skip = NewIntegrationTest(NewIntegrationTestArgs{ + Description: "Start a git bisect and skip a few commits (selected or current)", + ExtraCmdArgs: []string{}, + Skip: false, + SetupRepo: func(shell *Shell) { + shell. + CreateNCommits(10) + }, + SetupConfig: func(cfg *config.AppConfig) {}, + Run: func(t *TestDriver, keys config.KeybindingConfig) { + t.Views().Commits(). + Focus(). + SelectedLine(Contains("commit 10")). + Press(keys.Commits.ViewBisectOptions). + Tap(func() { + t.ExpectPopup().Menu().Title(Equals("Bisect")).Select(MatchesRegexp(`Mark .* as bad`)).Confirm() + }). + NavigateToLine(Contains("commit 01")). + Press(keys.Commits.ViewBisectOptions). + Tap(func() { + t.ExpectPopup().Menu().Title(Equals("Bisect")).Select(MatchesRegexp(`Mark .* as good`)).Confirm() + t.Views().Information().Content(Contains("Bisecting")) + }). + Lines( + Contains("CI commit 10").Contains("<-- bad"), + Contains("CI commit 09").DoesNotContain("<--"), + Contains("CI commit 08").DoesNotContain("<--"), + Contains("CI commit 07").DoesNotContain("<--"), + Contains("CI commit 06").DoesNotContain("<--"), + Contains("CI commit 05").Contains("<-- current").IsSelected(), + Contains("CI commit 04").DoesNotContain("<--"), + Contains("CI commit 03").DoesNotContain("<--"), + Contains("CI commit 02").DoesNotContain("<--"), + Contains("CI commit 01").Contains("<-- good"), + ). + Press(keys.Commits.ViewBisectOptions). + Tap(func() { + t.ExpectPopup().Menu().Title(Equals("Bisect")). + // Does not show a "Skip selected commit" entry: + Lines( + Contains("b Mark current commit").Contains("as bad"), + Contains("g Mark current commit").Contains("as good"), + Contains("s Skip current commit"), + Contains("r Reset bisect"), + Contains("Cancel"), + ). + Select(Contains("Skip current commit")).Confirm() + }). + // Skipping the current commit selects the new current commit: + Lines( + Contains("CI commit 10").Contains("<-- bad"), + Contains("CI commit 09").DoesNotContain("<--"), + Contains("CI commit 08").DoesNotContain("<--"), + Contains("CI commit 07").DoesNotContain("<--"), + Contains("CI commit 06").Contains("<-- current").IsSelected(), + Contains("CI commit 05").Contains("<-- skipped"), + Contains("CI commit 04").DoesNotContain("<--"), + Contains("CI commit 03").DoesNotContain("<--"), + Contains("CI commit 02").DoesNotContain("<--"), + Contains("CI commit 01").Contains("<-- good"), + ). + NavigateToLine(Contains("commit 07")). + Press(keys.Commits.ViewBisectOptions). + Tap(func() { + t.ExpectPopup().Menu().Title(Equals("Bisect")). + // Does show a "Skip selected commit" entry: + Lines( + Contains("b Mark current commit").Contains("as bad"), + Contains("g Mark current commit").Contains("as good"), + Contains("s Skip current commit"), + Contains("S Skip selected commit"), + Contains("r Reset bisect"), + Contains("Cancel"), + ). + Select(Contains("Skip selected commit")).Confirm() + }). + // Skipping a selected, non-current commit keeps the selection + // there: + SelectedLine(Contains("CI commit 07").Contains("<-- skipped")) + }, +}) diff --git a/pkg/integration/tests/test_list.go b/pkg/integration/tests/test_list.go index 17f01995c..88d1883ac 100644 --- a/pkg/integration/tests/test_list.go +++ b/pkg/integration/tests/test_list.go @@ -32,6 +32,7 @@ var tests = []*components.IntegrationTest{ bisect.Basic, bisect.ChooseTerms, bisect.FromOtherBranch, + bisect.Skip, branch.CheckoutByName, branch.CreateTag, branch.Delete,