summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorStefan Haller <stefan@haller-berlin.de>2024-03-02 10:20:23 +0100
committerGitHub <noreply@github.com>2024-03-02 10:20:23 +0100
commitee5533f9bf101e2ac313f9c0c41b280fb20d967c (patch)
tree822dc5c7486b556caed40dddf0a654b1f95cd51e
parented34ddc04d24e28603ddf8c7166ffc11db080f01 (diff)
parent416a40b0e69605399bde4b03a0ed48977da1dae8 (diff)
Don't show branch head on rebase todos if the rebase.updateRefs config is on (#3340)
- **PR Description** When doing an interactive rebase of a stack of branches with the `rebase.updateRefs` git config set to true, you get `update-ref` todos between the "pick" items. In this situation we would still show the branch head icon for each pick item that used to be the head of a branch. This is confusing, especially when you want to move a new commit to the head of a branch in the middle of the stack. Consider the following scenario: <img width="410" alt="image" src="https://github.com/jesseduffield/lazygit/assets/1225667/438b7157-e51e-48fb-95a9-b67039a7ad30"> Here we have made a new commit at the top of the stack, entered interactive rebase, and moved the commit down to the head of the second branch. Now it sits between the commit that shows the branch icon of the second branch, and the update-ref item for the second branch. This is super confusing, and it's simply better to not show branch icons for pick entries. That's what this PR does. We do show the icons if the `rebase.updateRefs` config is off, because otherwise there would be no indication of where the branches start and end. Of course, changing anything in this case will destroy the stack, so maybe it would be better to hide the icons in this case too to make this more obvious. I'm unsure about that.
-rw-r--r--pkg/gui/context/local_commits_context.go4
-rw-r--r--pkg/gui/context/sub_commits_context.go4
-rw-r--r--pkg/gui/presentation/commits.go12
-rw-r--r--pkg/integration/tests/interactive_rebase/dont_show_branch_heads_for_todo_items.go56
-rw-r--r--pkg/integration/tests/interactive_rebase/drop_todo_commit_with_update_ref.go2
-rw-r--r--pkg/integration/tests/interactive_rebase/quick_start_keep_selection.go2
-rw-r--r--pkg/integration/tests/interactive_rebase/quick_start_keep_selection_range.go4
-rw-r--r--pkg/integration/tests/interactive_rebase/view_files_of_todo_entries.go2
-rw-r--r--pkg/integration/tests/test_list.go1
9 files changed, 75 insertions, 12 deletions
diff --git a/pkg/gui/context/local_commits_context.go b/pkg/gui/context/local_commits_context.go
index cc166a8ec..604d51205 100644
--- a/pkg/gui/context/local_commits_context.go
+++ b/pkg/gui/context/local_commits_context.go
@@ -38,14 +38,14 @@ func NewLocalCommitsContext(c *ContextCommon) *LocalCommitsContext {
}
showYouAreHereLabel := c.Model().WorkingTreeStateAtLastCommitRefresh == enums.REBASE_MODE_REBASING
- showBranchMarkerForHeadCommit := c.Git().Config.GetRebaseUpdateRefs()
+ hasRebaseUpdateRefsConfig := c.Git().Config.GetRebaseUpdateRefs()
return presentation.GetCommitListDisplayStrings(
c.Common,
c.Model().Commits,
c.Model().Branches,
c.Model().CheckedOutBranch,
- showBranchMarkerForHeadCommit,
+ hasRebaseUpdateRefsConfig,
c.State().GetRepoState().GetScreenMode() != types.SCREEN_NORMAL,
c.Modes().CherryPicking.SelectedShaSet(),
c.Modes().Diffing.Ref,
diff --git a/pkg/gui/context/sub_commits_context.go b/pkg/gui/context/sub_commits_context.go
index 7a797e61d..d98188b91 100644
--- a/pkg/gui/context/sub_commits_context.go
+++ b/pkg/gui/context/sub_commits_context.go
@@ -55,13 +55,13 @@ func NewSubCommitsContext(
if viewModel.GetShowBranchHeads() {
branches = c.Model().Branches
}
- showBranchMarkerForHeadCommit := c.Git().Config.GetRebaseUpdateRefs()
+ hasRebaseUpdateRefsConfig := c.Git().Config.GetRebaseUpdateRefs()
return presentation.GetCommitListDisplayStrings(
c.Common,
c.Model().SubCommits,
branches,
viewModel.GetRef().RefName(),
- showBranchMarkerForHeadCommit,
+ hasRebaseUpdateRefsConfig,
c.State().GetRepoState().GetScreenMode() != types.SCREEN_NORMAL,
c.Modes().CherryPicking.SelectedShaSet(),
c.Modes().Diffing.Ref,
diff --git a/pkg/gui/presentation/commits.go b/pkg/gui/presentation/commits.go
index ac1006c3d..bd4057e41 100644
--- a/pkg/gui/presentation/commits.go
+++ b/pkg/gui/presentation/commits.go
@@ -41,7 +41,7 @@ func GetCommitListDisplayStrings(
commits []*models.Commit,
branches []*models.Branch,
currentBranchName string,
- showBranchMarkerForHeadCommit bool,
+ hasRebaseUpdateRefsConfig bool,
fullDescription bool,
cherryPickedCommitShaSet *set.Set[string],
diffName string,
@@ -123,7 +123,7 @@ func GetCommitListDisplayStrings(
!lo.Contains(common.UserConfig.Git.MainBranches, b.Name) &&
// Don't show a marker for the head commit unless the
// rebase.updateRefs config is on
- (showBranchMarkerForHeadCommit || b.CommitHash != commits[0].Sha)
+ (hasRebaseUpdateRefsConfig || b.CommitHash != commits[0].Sha)
}))
lines := make([][]string, 0, len(filteredCommits))
@@ -145,6 +145,7 @@ func GetCommitListDisplayStrings(
common,
commit,
branchHeadsToVisualize,
+ hasRebaseUpdateRefsConfig,
cherryPickedCommitShaSet,
isMarkedBaseCommit,
willBeRebased,
@@ -296,6 +297,7 @@ func displayCommit(
common *common.Common,
commit *models.Commit,
branchHeadsToVisualize *set.Set[string],
+ hasRebaseUpdateRefsConfig bool,
cherryPickedCommitShaSet *set.Set[string],
isMarkedBaseCommit bool,
willBeRebased bool,
@@ -329,7 +331,11 @@ func displayCommit(
tagString = theme.DiffTerminalColor.SetBold().Sprint(strings.Join(commit.Tags, " ")) + " "
}
- if branchHeadsToVisualize.Includes(commit.Sha) && commit.Status != models.StatusMerged {
+ if branchHeadsToVisualize.Includes(commit.Sha) &&
+ // Don't show branch head on commits that are already merged to a main branch
+ commit.Status != models.StatusMerged &&
+ // Don't show branch head on a "pick" todo if the rebase.updateRefs config is on
+ !(commit.IsTODO() && hasRebaseUpdateRefsConfig) {
tagString = style.FgCyan.SetBold().Sprint(
lo.Ternary(icons.IsIconEnabled(), icons.BRANCH_ICON, "*") + " " + tagString)
}
diff --git a/pkg/integration/tests/interactive_rebase/dont_show_branch_heads_for_todo_items.go b/pkg/integration/tests/interactive_rebase/dont_show_branch_heads_for_todo_items.go
new file mode 100644
index 000000000..7b433fa17
--- /dev/null
+++ b/pkg/integration/tests/interactive_rebase/dont_show_branch_heads_for_todo_items.go
@@ -0,0 +1,56 @@
+package interactive_rebase
+
+import (
+ "github.com/jesseduffield/lazygit/pkg/config"
+ . "github.com/jesseduffield/lazygit/pkg/integration/components"
+)
+
+var DontShowBranchHeadsForTodoItems = NewIntegrationTest(NewIntegrationTestArgs{
+ Description: "Check that branch heads are shown for normal commits during interactive rebase, but not for todo items",
+ ExtraCmdArgs: []string{},
+ Skip: false,
+ GitVersion: AtLeast("2.38.0"),
+ SetupConfig: func(config *config.AppConfig) {
+ config.AppState.GitLogShowGraph = "never"
+ },
+ SetupRepo: func(shell *Shell) {
+ shell.
+ NewBranch("branch1").
+ CreateNCommits(2).
+ NewBranch("branch2").
+ CreateNCommitsStartingAt(4, 3).
+ NewBranch("branch3").
+ CreateNCommitsStartingAt(3, 7)
+
+ shell.SetConfig("rebase.updateRefs", "true")
+ },
+ Run: func(t *TestDriver, keys config.KeybindingConfig) {
+ t.Views().Commits().
+ Focus().
+ Lines(
+ Contains("CI commit 09"),
+ Contains("CI commit 08"),
+ Contains("CI commit 07"),
+ Contains("CI * commit 06"),
+ Contains("CI commit 05"),
+ Contains("CI commit 04"),
+ Contains("CI commit 03"),
+ Contains("CI * commit 02"),
+ Contains("CI commit 01"),
+ ).
+ NavigateToLine(Contains("commit 04")).
+ Press(keys.Universal.Edit).
+ Lines(
+ Contains("pick").Contains("CI commit 09"),
+ Contains("pick").Contains("CI commit 08"),
+ Contains("pick").Contains("CI commit 07"),
+ Contains("update-ref").Contains("branch2"),
+ Contains("pick").Contains("CI commit 06"), // no star on this entry, even though branch2 points to it
+ Contains("pick").Contains("CI commit 05"),
+ Contains("CI <-- YOU ARE HERE --- commit 04"),
+ Contains("CI commit 03"),
+ Contains("CI * commit 02"), // this star is fine though
+ Contains("CI commit 01"),
+ )
+ },
+})
diff --git a/pkg/integration/tests/interactive_rebase/drop_todo_commit_with_update_ref.go b/pkg/integration/tests/interactive_rebase/drop_todo_commit_with_update_ref.go
index afc0fd073..94e851efa 100644
--- a/pkg/integration/tests/interactive_rebase/drop_todo_commit_with_update_ref.go
+++ b/pkg/integration/tests/interactive_rebase/drop_todo_commit_with_update_ref.go
@@ -44,7 +44,7 @@ var DropTodoCommitWithUpdateRef = NewIntegrationTest(NewIntegrationTestArgs{
Contains("pick").Contains("CI commit 06"),
Contains("pick").Contains("CI commit 05"),
Contains("update-ref").Contains("branch1").DoesNotContain("*"),
- Contains("pick").Contains("CI * commit 04"),
+ Contains("pick").Contains("CI commit 04"),
Contains("pick").Contains("CI commit 03"),
Contains("<-- YOU ARE HERE --- commit 02").IsSelected(),
Contains("CI commit 01"),
diff --git a/pkg/integration/tests/interactive_rebase/quick_start_keep_selection.go b/pkg/integration/tests/interactive_rebase/quick_start_keep_selection.go
index 2216b89b7..ba694b222 100644
--- a/pkg/integration/tests/interactive_rebase/quick_start_keep_selection.go
+++ b/pkg/integration/tests/interactive_rebase/quick_start_keep_selection.go
@@ -43,7 +43,7 @@ var QuickStartKeepSelection = NewIntegrationTest(NewIntegrationTestArgs{
Contains("pick").Contains("CI commit 06"),
Contains("pick").Contains("CI commit 05"),
Contains("update-ref").Contains("branch1"),
- Contains("pick").Contains("CI * commit 04"),
+ Contains("pick").Contains("CI commit 04"),
Contains("pick").Contains("CI commit 03"),
Contains("CI commit 02").IsSelected(),
Contains("CI <-- YOU ARE HERE --- commit 01"),
diff --git a/pkg/integration/tests/interactive_rebase/quick_start_keep_selection_range.go b/pkg/integration/tests/interactive_rebase/quick_start_keep_selection_range.go
index 20005ba6b..ea863e408 100644
--- a/pkg/integration/tests/interactive_rebase/quick_start_keep_selection_range.go
+++ b/pkg/integration/tests/interactive_rebase/quick_start_keep_selection_range.go
@@ -46,10 +46,10 @@ var QuickStartKeepSelectionRange = NewIntegrationTest(NewIntegrationTestArgs{
Contains("CI commit 07"),
Contains("CI commit 06"),
Contains("update-ref").Contains("branch2"),
- Contains("CI * commit 05"),
+ Contains("CI commit 05"),
Contains("CI commit 04").IsSelected(),
Contains("update-ref").Contains("branch1").IsSelected(),
- Contains("CI * commit 03").IsSelected(),
+ Contains("CI commit 03").IsSelected(),
Contains("CI commit 02").IsSelected(),
Contains("CI <-- YOU ARE HERE --- commit 01"),
)
diff --git a/pkg/integration/tests/interactive_rebase/view_files_of_todo_entries.go b/pkg/integration/tests/interactive_rebase/view_files_of_todo_entries.go
index 1b35abaaf..837c418d0 100644
--- a/pkg/integration/tests/interactive_rebase/view_files_of_todo_entries.go
+++ b/pkg/integration/tests/interactive_rebase/view_files_of_todo_entries.go
@@ -30,7 +30,7 @@ var ViewFilesOfTodoEntries = NewIntegrationTest(NewIntegrationTestArgs{
Lines(
Contains("pick").Contains("CI commit 03").IsSelected(),
Contains("update-ref").Contains("branch1"),
- Contains("pick").Contains("CI * commit 02"),
+ Contains("pick").Contains("CI commit 02"),
Contains("CI <-- YOU ARE HERE --- commit 01"),
).
Press(keys.Universal.GoInto)
diff --git a/pkg/integration/tests/test_list.go b/pkg/integration/tests/test_list.go
index e4efef40a..e26a0731f 100644
--- a/pkg/integration/tests/test_list.go
+++ b/pkg/integration/tests/test_list.go
@@ -162,6 +162,7 @@ var tests = []*components.IntegrationTest{
interactive_rebase.AmendHeadCommitDuringRebase,
interactive_rebase.AmendMerge,
interactive_rebase.AmendNonHeadCommitDuringRebase,
+ interactive_rebase.DontShowBranchHeadsForTodoItems,
interactive_rebase.DropTodoCommitWithUpdateRef,
interactive_rebase.DropWithCustomCommentChar,
interactive_rebase.EditFirstCommit,