Repository: jesseduffield/lazygit (language: go)

Issue:
Bisect good/bad: current or selected commit?
When I'm in the middle of a bisect, and I want to mark a commit as good or bad, I always want to mark the _current_ commit. Not the selected commit. In 100% of the cases.

There are two common reasons for me why the current commit is not the selected one: 
- I moved around a bit (a few commits up/down) to see if I can quickly spot the bad commit myself, without having to continue the bisect. Then I forget to move the selection back to the current commit.
- I quit and restart lazygit. It doesn't select the current commit in this case (might be nice if it did).

Typing `b b` or `b g` in these cases doesn't do what I want, and I managed to confuse myself badly several times because of this. I expect these to behave as if I typed `git bisect good` or `git bisect bad` on the command line.

I noticed the following comment in the source code:
```
	// Originally we were allowing the user to, from the bisect menu, select whether
	// they were talking about the selected commit or the current bisect commit,
	// and that was a bit confusing (and required extra keypresses).
```

I couldn't find any PR where this was discussed; blaming this comment, it looks like it was there from the beginning (4ab5e5413944), so I'm not sure what the "Originally" is referring to.

My proposal is to change the behavior to unconditionally work on the current commit instead of the selected one; I claim that this is what everybody expects/wants. Alternatively, if this is deemed too much of a change, we could show a menu like this:
```
  b Mark current commit (deadbeef) as bad
  g Mark current commit (deadbeef) as good
  B Mark selected commit (fa1afe1) as bad
  G Mark selected commit (fa1afe1) as good
  s Skip current commit (deadbeef) 
  r Reset bisect
    Cancel
```

We would include the 3rd and 4th menu entries only when the selected commit is not the current one, so for the typical usage nothing would change.


Discussion / hints:
Oops, found this discussion meanwhile: https://github.com/jesseduffield/lazygit/pull/1704#issuecomment-1018969351

Doesn't change anything for me though...
OK, after re-reading the discussion in #1704, it seems that you want the ability to _skip_ a selected commit that is not the current one. This makes sense to me, but I still think that marking selected (non-current) commits as good/bad doesn't.

So the menu would have to look something like this then:
```
  b Mark current commit (deadbeef) as bad
  g Mark current commit (deadbeef) as good
  s Skip current commit (deadbeef) 
  S Skip selected commit (fa1afe1) 
  r Reset bisect
    Cancel
```

Again, the `S` entry would only appear when the selected commit is not the current one.

Thoughts?
I'm happy with that. I never need to mark the non-current commit as good/bad either. If somebody out there does have that use case, we can add additional menu items for that (after they raise an issue for it)

Fix patch:
diff --git a/pkg/gui/controllers/bisect_controller.go b/pkg/gui/controllers/bisect_controller.go
index afa417bb131..7cb36ef26bd 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,10 +99,10 @@ 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(commit.Sha); err != nil {
+				if err := self.c.Git().Bisect.Skip(shaToMark); err != nil {
 					return self.c.Error(err)
 				}
 
@@ -102,14 +110,28 @@ func (self *BisectController) openMidBisectMenu(info *git_commands.BisectInfo, c
 			},
 			Key: 's',
 		},
-		{
-			Label: self.c.Tr.Bisect.ResetOption,
+	}
+	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 {
-				return self.c.Helpers().Bisect.Reset()
+				self.c.LogAction(self.c.Tr.Actions.BisectSkip)
+				if err := self.c.Git().Bisect.Skip(commit.Sha); err != nil {
+					return self.c.Error(err)
+				}
+
+				return self.afterMark(selectCurrentAfter, waitToReselect)
 			},
-			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 f6171f68e5f..2a494c97efd 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 2d0fb90e809..34f9e66e0bb 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 6f77aa26da7..3cbc6e225ae 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 801a83474a6..f22e6cc4728 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 fe4785667db..ae21430e6b8 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 ebdda62bf28..3ccac945a4e 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:                 "重設二分查找",


Test patch:
diff --git a/pkg/integration/tests/bisect/skip.go b/pkg/integration/tests/bisect/skip.go
new file mode 100644
index 00000000000..998e82361df
--- /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 17f01995cb1..88d1883acd2 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,