From 5b613f5bc78c44351945cbc3ffdf9e5950c04bd8 Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Thu, 2 May 2024 20:05:16 +0200 Subject: [PATCH] Remove ColoredBranchStatus and branchStatusColor MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously the entire status was colored in a single color, so the API made sense. This is going to change in the next commit, so now we must include the color in the string returned from BranchStatus(), which means that callers who need to do hit detection or measure the length need to decolorize it. While we're at it, switch the order of ↑3↓7 to ↓7↑3. For some reason that I can't really explain I find it more logical this way. The software out there is pretty undecided about it, it seems: VS Code puts ↓7 first, and so does the shell prompt that comes with git; git status and git branch -v put "ahead" first though. Shrug. --- pkg/gui/controllers/status_controller.go | 3 +- pkg/gui/presentation/branches.go | 49 +++++-------------- pkg/gui/presentation/branches_test.go | 4 +- pkg/gui/presentation/status.go | 6 +-- pkg/integration/tests/sync/pull_merge.go | 2 +- .../tests/sync/pull_merge_conflict.go | 2 +- pkg/integration/tests/sync/pull_rebase.go | 2 +- .../tests/sync/pull_rebase_conflict.go | 2 +- .../sync/pull_rebase_interactive_conflict.go | 2 +- .../pull_rebase_interactive_conflict_drop.go | 2 +- 10 files changed, 26 insertions(+), 48 deletions(-) diff --git a/pkg/gui/controllers/status_controller.go b/pkg/gui/controllers/status_controller.go index 4c4384bfd..483acdda6 100644 --- a/pkg/gui/controllers/status_controller.go +++ b/pkg/gui/controllers/status_controller.go @@ -12,6 +12,7 @@ import ( "github.com/jesseduffield/lazygit/pkg/gui/presentation" "github.com/jesseduffield/lazygit/pkg/gui/style" "github.com/jesseduffield/lazygit/pkg/gui/types" + "github.com/jesseduffield/lazygit/pkg/utils" "github.com/samber/lo" ) @@ -116,7 +117,7 @@ func (self *StatusController) onClick(opts gocui.ViewMouseBindingOpts) error { return err } - upstreamStatus := presentation.BranchStatus(currentBranch, types.ItemOperationNone, self.c.Tr, time.Now(), self.c.UserConfig) + upstreamStatus := utils.Decolorise(presentation.BranchStatus(currentBranch, types.ItemOperationNone, self.c.Tr, time.Now(), self.c.UserConfig)) repoName := self.c.Git().RepoPaths.RepoName() workingTreeState := self.c.Git().Status.WorkingTreeState() switch workingTreeState { diff --git a/pkg/gui/presentation/branches.go b/pkg/gui/presentation/branches.go index 406a580d5..0e48a3935 100644 --- a/pkg/gui/presentation/branches.go +++ b/pkg/gui/presentation/branches.go @@ -56,7 +56,7 @@ func getBranchDisplayStrings( // Recency is always three characters, plus one for the space availableWidth := viewWidth - 4 if len(branchStatus) > 0 { - availableWidth -= runewidth.StringWidth(branchStatus) + 1 + availableWidth -= runewidth.StringWidth(utils.Decolorise(branchStatus)) + 1 } if icons.IsIconEnabled() { availableWidth -= 2 // one for the icon, one for the space @@ -89,8 +89,7 @@ func getBranchDisplayStrings( coloredName = fmt.Sprintf("%s %s", coloredName, style.FgDefault.Sprint(worktreeIcon)) } if len(branchStatus) > 0 { - coloredStatus := branchStatusColor(b, itemOperation).Sprint(branchStatus) - coloredName = fmt.Sprintf("%s %s", coloredName, coloredStatus) + coloredName = fmt.Sprintf("%s %s", coloredName, branchStatus) } recencyColor := style.FgCyan @@ -144,30 +143,6 @@ func GetBranchTextStyle(name string) style.TextStyle { } } -func branchStatusColor(branch *models.Branch, itemOperation types.ItemOperation) style.TextStyle { - colour := style.FgYellow - if itemOperation != types.ItemOperationNone { - colour = style.FgCyan - } else if branch.UpstreamGone { - colour = style.FgRed - } else if branch.MatchesUpstream() { - colour = style.FgGreen - } else if branch.RemoteBranchNotStoredLocally() { - colour = style.FgMagenta - } - - return colour -} - -func ColoredBranchStatus( - branch *models.Branch, - itemOperation types.ItemOperation, - tr *i18n.TranslationSet, - userConfig *config.UserConfig, -) string { - return branchStatusColor(branch, itemOperation).Sprint(BranchStatus(branch, itemOperation, tr, time.Now(), userConfig)) -} - func BranchStatus( branch *models.Branch, itemOperation types.ItemOperation, @@ -177,7 +152,7 @@ func BranchStatus( ) string { itemOperationStr := ItemOperationToString(itemOperation, tr) if itemOperationStr != "" { - return itemOperationStr + " " + utils.Loader(now, userConfig.Gui.Spinner) + return style.FgCyan.Sprintf("%s %s", itemOperationStr, utils.Loader(now, userConfig.Gui.Spinner)) } if !branch.IsTrackingRemote() { @@ -185,25 +160,27 @@ func BranchStatus( } if branch.UpstreamGone { - return tr.UpstreamGone + return style.FgRed.Sprint(tr.UpstreamGone) } if branch.MatchesUpstream() { - return "✓" + return style.FgGreen.Sprint("✓") } if branch.RemoteBranchNotStoredLocally() { - return "?" + return style.FgMagenta.Sprint("?") } - result := "" - if branch.IsAheadForPull() { - result = fmt.Sprintf("↑%s", branch.AheadForPull) + if branch.IsBehindForPull() && branch.IsAheadForPull() { + return style.FgYellow.Sprintf("↓%s↑%s", branch.BehindForPull, branch.AheadForPull) } if branch.IsBehindForPull() { - result = fmt.Sprintf("%s↓%s", result, branch.BehindForPull) + return style.FgYellow.Sprintf("↓%s", branch.BehindForPull) + } + if branch.IsAheadForPull() { + return style.FgYellow.Sprintf("↑%s", branch.AheadForPull) } - return result + return "" } func SetCustomBranches(customBranchColors map[string]string) { diff --git a/pkg/gui/presentation/branches_test.go b/pkg/gui/presentation/branches_test.go index cf2f1d994..db4868970 100644 --- a/pkg/gui/presentation/branches_test.go +++ b/pkg/gui/presentation/branches_test.go @@ -81,7 +81,7 @@ func Test_getBranchDisplayStrings(t *testing.T) { viewWidth: 100, useIcons: false, checkedOutByWorktree: true, - expected: []string{"1m", "branch_name (worktree) ↑3↓5"}, + expected: []string{"1m", "branch_name (worktree) ↓5↑3"}, }, { branch: &models.Branch{Name: "branch_name", Recency: "1m"}, @@ -167,7 +167,7 @@ func Test_getBranchDisplayStrings(t *testing.T) { viewWidth: 30, useIcons: false, checkedOutByWorktree: true, - expected: []string{"1m", "branch_na… (worktree) ↑3↓5"}, + expected: []string{"1m", "branch_na… (worktree) ↓5↑3"}, }, { branch: &models.Branch{Name: "branch_name", Recency: "1m"}, diff --git a/pkg/gui/presentation/status.go b/pkg/gui/presentation/status.go index 5ac591c48..b3b210067 100644 --- a/pkg/gui/presentation/status.go +++ b/pkg/gui/presentation/status.go @@ -2,6 +2,7 @@ package presentation import ( "fmt" + "time" "github.com/jesseduffield/lazygit/pkg/commands/models" "github.com/jesseduffield/lazygit/pkg/commands/types/enums" @@ -10,7 +11,6 @@ import ( "github.com/jesseduffield/lazygit/pkg/gui/style" "github.com/jesseduffield/lazygit/pkg/gui/types" "github.com/jesseduffield/lazygit/pkg/i18n" - "github.com/jesseduffield/lazygit/pkg/utils" ) func FormatStatus( @@ -25,8 +25,8 @@ func FormatStatus( status := "" if currentBranch.IsRealBranch() { - status += ColoredBranchStatus(currentBranch, itemOperation, tr, userConfig) - if utils.Decolorise(status) != "" { + status += BranchStatus(currentBranch, itemOperation, tr, time.Now(), userConfig) + if status != "" { status += " " } } diff --git a/pkg/integration/tests/sync/pull_merge.go b/pkg/integration/tests/sync/pull_merge.go index d4809f00e..39e447ebc 100644 --- a/pkg/integration/tests/sync/pull_merge.go +++ b/pkg/integration/tests/sync/pull_merge.go @@ -33,7 +33,7 @@ var PullMerge = NewIntegrationTest(NewIntegrationTestArgs{ Contains("one"), ) - t.Views().Status().Content(Equals("↑1↓2 repo → master")) + t.Views().Status().Content(Equals("↓2↑1 repo → master")) t.Views().Files(). IsFocused(). diff --git a/pkg/integration/tests/sync/pull_merge_conflict.go b/pkg/integration/tests/sync/pull_merge_conflict.go index 6e05fafae..2161f6abd 100644 --- a/pkg/integration/tests/sync/pull_merge_conflict.go +++ b/pkg/integration/tests/sync/pull_merge_conflict.go @@ -34,7 +34,7 @@ var PullMergeConflict = NewIntegrationTest(NewIntegrationTestArgs{ Contains("one"), ) - t.Views().Status().Content(Equals("↑1↓2 repo → master")) + t.Views().Status().Content(Equals("↓2↑1 repo → master")) t.Views().Files(). IsFocused(). diff --git a/pkg/integration/tests/sync/pull_rebase.go b/pkg/integration/tests/sync/pull_rebase.go index 158cdf3d3..a2657ffe6 100644 --- a/pkg/integration/tests/sync/pull_rebase.go +++ b/pkg/integration/tests/sync/pull_rebase.go @@ -35,7 +35,7 @@ var PullRebase = NewIntegrationTest(NewIntegrationTestArgs{ Contains("one"), ) - t.Views().Status().Content(Equals("↑1↓2 repo → master")) + t.Views().Status().Content(Equals("↓2↑1 repo → master")) t.Views().Files(). IsFocused(). diff --git a/pkg/integration/tests/sync/pull_rebase_conflict.go b/pkg/integration/tests/sync/pull_rebase_conflict.go index fa2920b7b..d9541e0ed 100644 --- a/pkg/integration/tests/sync/pull_rebase_conflict.go +++ b/pkg/integration/tests/sync/pull_rebase_conflict.go @@ -34,7 +34,7 @@ var PullRebaseConflict = NewIntegrationTest(NewIntegrationTestArgs{ Contains("one"), ) - t.Views().Status().Content(Equals("↑1↓2 repo → master")) + t.Views().Status().Content(Equals("↓2↑1 repo → master")) t.Views().Files(). IsFocused(). diff --git a/pkg/integration/tests/sync/pull_rebase_interactive_conflict.go b/pkg/integration/tests/sync/pull_rebase_interactive_conflict.go index e93ca2e37..bf0fc050b 100644 --- a/pkg/integration/tests/sync/pull_rebase_interactive_conflict.go +++ b/pkg/integration/tests/sync/pull_rebase_interactive_conflict.go @@ -38,7 +38,7 @@ var PullRebaseInteractiveConflict = NewIntegrationTest(NewIntegrationTestArgs{ Contains("one"), ) - t.Views().Status().Content(Equals("↑2↓2 repo → master")) + t.Views().Status().Content(Equals("↓2↑2 repo → master")) t.Views().Files(). IsFocused(). diff --git a/pkg/integration/tests/sync/pull_rebase_interactive_conflict_drop.go b/pkg/integration/tests/sync/pull_rebase_interactive_conflict_drop.go index eba3ba746..3eee12efd 100644 --- a/pkg/integration/tests/sync/pull_rebase_interactive_conflict_drop.go +++ b/pkg/integration/tests/sync/pull_rebase_interactive_conflict_drop.go @@ -38,7 +38,7 @@ var PullRebaseInteractiveConflictDrop = NewIntegrationTest(NewIntegrationTestArg Contains("one"), ) - t.Views().Status().Content(Equals("↑2↓2 repo → master")) + t.Views().Status().Content(Equals("↓2↑2 repo → master")) t.Views().Files(). IsFocused().