| Message ID | patch-3.4-937e728f7f-20210617T105245Z-avarab@gmail.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 4465690cd838017208d297a7f77d00ec51ccf7fd |
| Headers | show |
| Series | show-branch: add missing tests, trivial color output change | expand |
Ævar Arnfjörð Bjarmason wrote: > Change the colored output introduced in ab07ba2a24 (show-branch: color > the commit status signs, 2009-04-22) to not color and reset each > individual space character we use for padding. The intent is to color > just the "!", "+" etc. characters. Obviously a fix, but perhaps show the current behavior: <RED>+<RESET><GREEN> <RESET><YELLOW> <RESET><BLUE> <RESET><MAGENTA> <RESET><CYAN> <RESET><BOLD;RED> <RESET><BOLD;GREEN> <RESET><BOLD;YELLOW> <RESET><BOLD;BLUE> <RESET> [branch1] branch1 Versus: <RED>+<RESET> [branch1] branch1 > In theory this breaks things for anyone who's relying on the exact > colored output of show-branch, in practice I'd think anyone parsing it > isn't actively turning on the colored output. Please let's limit our worries to real users. Few people use `git show-branch`, even less would rely on parsing its bogus output. > --- a/builtin/show-branch.c > +++ b/builtin/show-branch.c > @@ -939,9 +939,12 @@ int cmd_show_branch(int ac, const char **av, const char *prefix) > mark = '*'; > else > mark = '+'; > - printf("%s%c%s", > - get_color_code(i), > - mark, get_color_reset_code()); > + if (mark == ' ') > + putchar(mark); > + else > + printf("%s%c%s", > + get_color_code(i), > + mark, get_color_reset_code()); Very simple and obvious improvement. Cheers.
diff --git a/builtin/show-branch.c b/builtin/show-branch.c index d6d2dabeca..d77ce7aeb3 100644 --- a/builtin/show-branch.c +++ b/builtin/show-branch.c @@ -939,9 +939,12 @@ int cmd_show_branch(int ac, const char **av, const char *prefix) mark = '*'; else mark = '+'; - printf("%s%c%s", - get_color_code(i), - mark, get_color_reset_code()); + if (mark == ' ') + putchar(mark); + else + printf("%s%c%s", + get_color_code(i), + mark, get_color_reset_code()); } putchar(' '); } diff --git a/t/t3202-show-branch.sh b/t/t3202-show-branch.sh index 7b06048905..54025f0337 100755 --- a/t/t3202-show-branch.sh +++ b/t/t3202-show-branch.sh @@ -55,4 +55,34 @@ test_expect_success 'show-branch with showbranch.default' ' test_cmp expect actual ' +test_expect_success 'show-branch --color output' ' + sed "s/^> //" >expect <<-\EOF && + > <RED>!<RESET> [branch1] branch1 + > <GREEN>!<RESET> [branch2] branch2 + > <YELLOW>!<RESET> [branch3] branch3 + > <BLUE>!<RESET> [branch4] branch4 + > <MAGENTA>!<RESET> [branch5] branch5 + > <CYAN>!<RESET> [branch6] branch6 + > <BOLD;RED>!<RESET> [branch7] branch7 + > <BOLD;GREEN>!<RESET> [branch8] branch8 + > <BOLD;YELLOW>!<RESET> [branch9] branch9 + > <BOLD;BLUE>*<RESET> [branch10] branch10 + > ---------- + > <BOLD;BLUE>*<RESET> [branch10] branch10 + > <BOLD;YELLOW>+<RESET> [branch9] branch9 + > <BOLD;GREEN>+<RESET> [branch8] branch8 + > <BOLD;RED>+<RESET> [branch7] branch7 + > <CYAN>+<RESET> [branch6] branch6 + > <MAGENTA>+<RESET> [branch5] branch5 + > <BLUE>+<RESET> [branch4] branch4 + > <YELLOW>+<RESET> [branch3] branch3 + > <GREEN>+<RESET> [branch2] branch2 + > <RED>+<RESET> [branch1] branch1 + > <RED>+<RESET><GREEN>+<RESET><YELLOW>+<RESET><BLUE>+<RESET><MAGENTA>+<RESET><CYAN>+<RESET><BOLD;RED>+<RESET><BOLD;GREEN>+<RESET><BOLD;YELLOW>+<RESET><BOLD;BLUE>*<RESET> [branch10^] initial + EOF + git show-branch --color=always $(cat branches.sorted) >actual.raw && + test_decode_color <actual.raw >actual && + test_cmp expect actual +' + test_done
Change the colored output introduced in ab07ba2a24 (show-branch: color the commit status signs, 2009-04-22) to not color and reset each individual space character we use for padding. The intent is to color just the "!", "+" etc. characters. This makes the output easier to test, so let's do that now. The test would be much more verbose without a color/reset for each space character. Since the coloring cycles through colors we previously had a "rainbow of space characters". In theory this breaks things for anyone who's relying on the exact colored output of show-branch, in practice I'd think anyone parsing it isn't actively turning on the colored output. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> --- builtin/show-branch.c | 9 ++++++--- t/t3202-show-branch.sh | 30 ++++++++++++++++++++++++++++++ 2 files changed, 36 insertions(+), 3 deletions(-)