[v2,0/4] show-branch: add missing tests, trivial color output change

Message

Ævar Arnfjörð Bjarmason June 17, 2021, 10:53 a.m. UTC

This v2 doesn't change any of the code (see range-diff), but better
explains the change per Michael J Gruber's feedback in
https://lore.kernel.org/git/[e-mail]/

There's also a trivial grammar fix, s/add/odd/g.

Ævar Arnfjörð Bjarmason (4):
  show-branch tests: rename the one "show-branch" test file
  show-branch tests: modernize test code
  show-branch: don't <COLOR></RESET> for space characters
  show-branch tests: add missing tests

 builtin/show-branch.c          |   9 +-
 t/t3202-show-branch-octopus.sh |  70 ----------------
 t/t3202-show-branch.sh         | 149 +++++++++++++++++++++++++++++++++
 3 files changed, 155 insertions(+), 73 deletions(-)
 delete mode 100755 t/t3202-show-branch-octopus.sh
 create mode 100755 t/t3202-show-branch.sh

Range-diff against v1:
1:  7b8ac43339 = 1:  7b8ac43339 show-branch tests: rename the one "show-branch" test file
2:  27f94abaed = 2:  27f94abaed show-branch tests: modernize test code
3:  8db7029086 ! 3:  937e728f7f show-branch: fix and test --color output
    @@ Metadata
     Author: Ævar Arnfjörð Bjarmason <[e-mail]>
     
      ## Commit message ##
    -    show-branch: fix and test --color output
    +    show-branch: don't <COLOR></RESET> for space characters
     
    -    Fix the "show-branch --color" output so it doesn't needlessly color
    -    and reset each time it emits a space character.
    +    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 <[e-mail]>
     
4:  c54c6a7b20 ! 4:  dde0177235 show-branch tests: add missing tests
    @@ Commit message
         This fixes a few more blind spots, but there's still a lot of behavior
         that's not tested for.
     
    -    These new tests show the add (and possibly unintentional) behavior of
    +    These new tests show the odd (and possibly unintentional) behavior of
         --merge-base with one argument, and how its output is the same as "git
         merge-base" with N bases in this particular case. See the test added
         in f621a8454d1 (git-merge-base/git-show-branch --merge-base:

Comments

Felipe Contreras June 17, 2021, 9:46 p.m. UTC | #1

Ævar Arnfjörð Bjarmason wrote:
> This v2 doesn't change any of the code (see range-diff), but better
> explains the change per Michael J Gruber's feedback in
> https://lore.kernel.org/git/[e-mail]/
> 
> There's also a trivial grammar fix, s/add/odd/g.
> 
> Ævar Arnfjörð Bjarmason (4):
>   show-branch tests: rename the one "show-branch" test file
>   show-branch tests: modernize test code
>   show-branch: don't <COLOR></RESET> for space characters
>   show-branch tests: add missing tests

All these look good to me (I would prefer patch #2 to be split into
multiple patches, but that's not a deal-breaker).

Reviewed-by: Felipe Contreras <[e-mail]>