From 883c536c88a42b997a246628a2033380ac38b46e Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 31 Jul 2025 10:58:34 +0000 Subject: [PATCH] Address review feedback: table alignment, refactor to appendFlag, add combination tests Co-authored-by: arl <476650+arl@users.noreply.github.com> --- README.md | 18 ++--- tmux/formater.go | 50 +++++-------- tmux/formater_test.go | 158 ++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 184 insertions(+), 42 deletions(-) diff --git a/README.md b/README.md index 480af03..6824e57 100644 --- a/README.md +++ b/README.md @@ -288,15 +288,15 @@ layout: [branch, "|", flags, "|", stats] This is the list of additional configuration `options`: -| Option | Description | Default | -| :------------------------------ | :------------------------------------------------------------------------------ | :----------------: | -| `branch_max_len` | Maximum displayed length for local and remote branch names | `0` (no limit) | -| `branch_trim` | Trim left, right or from the center of the branch (`right`, `left` or `center`) | `right` (trailing) | -| `ellipsis` | Character to show branch name has been truncated | `…` | -| `hide_clean` | Hides the clean flag entirely | `false` | -| `swap_divergence` | Swaps order of behind & ahead upstream counts | `false` | -| `divergence_space` | Add a space between behind & ahead upstream counts | `false` | -| `flags_without_count` | Show flags symbols without counts | `false` | +| Option | Description | Default | +| :-------------------------------- | :------------------------------------------------------------------------------ | :----------------: | +| `branch_max_len` | Maximum displayed length for local and remote branch names | `0` (no limit) | +| `branch_trim` | Trim left, right or from the center of the branch (`right`, `left` or `center`) | `right` (trailing) | +| `ellipsis` | Character to show branch name has been truncated | `…` | +| `hide_clean` | Hides the clean flag entirely | `false` | +| `swap_divergence` | Swaps order of behind & ahead upstream counts | `false` | +| `divergence_space` | Add a space between behind & ahead upstream counts | `false` | +| `flags_without_count` | Show flags symbols without counts | `false` | | `hide_flag_count_if_empty_symbol` | Hide flag count when symbol is empty (false shows count only) | `false` | ## Troubleshooting diff --git a/tmux/formater.go b/tmux/formater.go index 25dc252..20c2ffc 100644 --- a/tmux/formater.go +++ b/tmux/formater.go @@ -297,40 +297,39 @@ func (f *Formater) currentRef() string { } // formatFlag formats a flag with or without count based on the flags_without_count option -func (f *Formater) formatFlag(style, symbol string, count int) string { +func (f *Formater) appendFlag(flags []string, style, symbol string, count int) []string { // Handle empty symbol case based on hide_flag_count_if_empty_symbol option if symbol == "" { if f.Options.HideFlagCountIfEmptySymbol { - return "" // Hide both symbol and count + return flags // Hide both symbol and count } // Show just the count without symbol - return fmt.Sprintf("%s%d", style, count) + return append(flags, fmt.Sprintf("%s%d", style, count)) } + // Handle flags_without_count option if f.Options.FlagsWithoutCount { - return fmt.Sprintf("%s%s", style, symbol) + return append(flags, fmt.Sprintf("%s%s", style, symbol)) } - return fmt.Sprintf("%s%s%d", style, symbol, count) + + // Default behavior: show symbol and count + return append(flags, fmt.Sprintf("%s%s%d", style, symbol, count)) } func (f *Formater) flags() string { var flags []string if f.st.IsClean { if f.st.NumStashed != 0 { - flag := f.formatFlag(f.Styles.Stashed, f.Symbols.Stashed, f.st.NumStashed) - if flag != "" { - flags = append(flags, flag) - } + flags = f.appendFlag(flags, f.Styles.Stashed, f.Symbols.Stashed, f.st.NumStashed) } if !f.Options.HideClean { - // Handle clean symbol separately since it doesn't have a count + // Handle clean symbol separately since it doesn't have a meaningful count if f.Symbols.Clean != "" { flags = append(flags, fmt.Sprintf("%s%s", f.Styles.Clean, f.Symbols.Clean)) - } else if !f.Options.HideFlagCountIfEmptySymbol { - // When symbol is empty but we don't want to hide, there's no count to show for clean flag - // so we just skip it (nothing meaningful to display) } + // Note: When clean symbol is empty, there's nothing meaningful to show + // since clean doesn't have a count, so we just skip it } if len(flags) != 0 { @@ -339,38 +338,23 @@ func (f *Formater) flags() string { } if f.st.NumStaged != 0 { - flag := f.formatFlag(f.Styles.Staged, f.Symbols.Staged, f.st.NumStaged) - if flag != "" { - flags = append(flags, flag) - } + flags = f.appendFlag(flags, f.Styles.Staged, f.Symbols.Staged, f.st.NumStaged) } if f.st.NumConflicts != 0 { - flag := f.formatFlag(f.Styles.Conflict, f.Symbols.Conflict, f.st.NumConflicts) - if flag != "" { - flags = append(flags, flag) - } + flags = f.appendFlag(flags, f.Styles.Conflict, f.Symbols.Conflict, f.st.NumConflicts) } if f.st.NumModified != 0 { - flag := f.formatFlag(f.Styles.Modified, f.Symbols.Modified, f.st.NumModified) - if flag != "" { - flags = append(flags, flag) - } + flags = f.appendFlag(flags, f.Styles.Modified, f.Symbols.Modified, f.st.NumModified) } if f.st.NumStashed != 0 { - flag := f.formatFlag(f.Styles.Stashed, f.Symbols.Stashed, f.st.NumStashed) - if flag != "" { - flags = append(flags, flag) - } + flags = f.appendFlag(flags, f.Styles.Stashed, f.Symbols.Stashed, f.st.NumStashed) } if f.st.NumUntracked != 0 { - flag := f.formatFlag(f.Styles.Untracked, f.Symbols.Untracked, f.st.NumUntracked) - if flag != "" { - flags = append(flags, flag) - } + flags = f.appendFlag(flags, f.Styles.Untracked, f.Symbols.Untracked, f.st.NumUntracked) } if len(flags) > 0 { diff --git a/tmux/formater_test.go b/tmux/formater_test.go index 0a04556..26ba521 100644 --- a/tmux/formater_test.go +++ b/tmux/formater_test.go @@ -1299,6 +1299,164 @@ func TestFlagsWithEmptySymbolsNewBehavior(t *testing.T) { } } +func TestFlagsWithCombinedOptions(t *testing.T) { + tests := []struct { + name string + styles styles + symbols symbols + options options + st *gitstatus.Status + want string + }{ + { + name: "flags_without_count=true + hide_flag_count_if_empty_symbol=false, empty symbol shows nothing", + styles: styles{ + Clear: "StyleClear", + Modified: "StyleMod", + Stashed: "StyleStash", + }, + symbols: symbols{ + Modified: "", // empty symbol + Stashed: "SymbolStash", + }, + options: options{ + FlagsWithoutCount: true, + HideFlagCountIfEmptySymbol: false, // Should show count only for empty symbols + }, + st: &gitstatus.Status{ + NumStashed: 1, + Porcelain: gitstatus.Porcelain{ + NumModified: 2, + }, + }, + // When flags_without_count=true but symbol is empty, we still get count-only display + want: "StyleClear" + "StyleMod2 StyleStashSymbolStash", + }, + { + name: "flags_without_count=true + hide_flag_count_if_empty_symbol=true, empty symbol hides everything", + styles: styles{ + Clear: "StyleClear", + Modified: "StyleMod", + Stashed: "StyleStash", + }, + symbols: symbols{ + Modified: "", // empty symbol + Stashed: "SymbolStash", + }, + options: options{ + FlagsWithoutCount: true, + HideFlagCountIfEmptySymbol: true, // Should hide empty symbols completely + }, + st: &gitstatus.Status{ + NumStashed: 1, + Porcelain: gitstatus.Porcelain{ + NumModified: 2, + }, + }, + // Empty symbol should be hidden entirely + want: "StyleClear" + "StyleStashSymbolStash", + }, + { + name: "flags_without_count=false + hide_flag_count_if_empty_symbol=false, mixed symbols", + styles: styles{ + Clear: "StyleClear", + Modified: "StyleMod", + Stashed: "StyleStash", + Untracked: "StyleUntracked", + }, + symbols: symbols{ + Modified: "", // empty symbol + Stashed: "SymbolStash", + Untracked: "", // empty symbol + }, + options: options{ + FlagsWithoutCount: false, + HideFlagCountIfEmptySymbol: false, // Show count for empty symbols + }, + st: &gitstatus.Status{ + NumStashed: 1, + Porcelain: gitstatus.Porcelain{ + NumModified: 2, + NumUntracked: 3, + }, + }, + // Empty symbols show count only, normal symbols show symbol+count + want: "StyleClear" + "StyleMod2 StyleStashSymbolStash1 StyleUntracked3", + }, + { + name: "flags_without_count=false + hide_flag_count_if_empty_symbol=true, mixed symbols", + styles: styles{ + Clear: "StyleClear", + Modified: "StyleMod", + Stashed: "StyleStash", + Untracked: "StyleUntracked", + }, + symbols: symbols{ + Modified: "", // empty symbol + Stashed: "SymbolStash", + Untracked: "", // empty symbol + }, + options: options{ + FlagsWithoutCount: false, + HideFlagCountIfEmptySymbol: true, // Hide empty symbols completely + }, + st: &gitstatus.Status{ + NumStashed: 1, + Porcelain: gitstatus.Porcelain{ + NumModified: 2, + NumUntracked: 3, + }, + }, + // Empty symbols are hidden, normal symbols show symbol+count + want: "StyleClear" + "StyleStashSymbolStash1", + }, + { + name: "all flags with different combination settings", + styles: styles{ + Clear: "StyleClear", + Staged: "StyleStaged", + Modified: "StyleMod", + Conflict: "StyleConflict", + Stashed: "StyleStash", + Untracked: "StyleUntracked", + }, + symbols: symbols{ + Staged: "SymbolStaged", + Modified: "", // empty + Conflict: "SymbolConflict", + Stashed: "", // empty + Untracked: "SymbolUntracked", + }, + options: options{ + FlagsWithoutCount: true, // Show symbols without counts + HideFlagCountIfEmptySymbol: false, // But for empty symbols, show count only + }, + st: &gitstatus.Status{ + NumStashed: 2, + Porcelain: gitstatus.Porcelain{ + NumStaged: 1, + NumModified: 3, + NumConflicts: 1, + NumUntracked: 4, + }, + }, + // Non-empty symbols show symbol only (flags_without_count=true) + // Empty symbols show count only (hide_flag_count_if_empty_symbol=false) + want: "StyleClear" + "StyleStagedSymbolStaged StyleConflictSymbolConflict StyleMod3 StyleStash2 StyleUntrackedSymbolUntracked", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + f := &Formater{ + Config: Config{Styles: tt.styles, Symbols: tt.symbols, Options: tt.options}, + st: tt.st, + } + + compareStrings(t, tt.want, f.flags()) + }) + } +} + func compareStrings(t *testing.T, want, got string) { if got != want { t.Errorf(`