{"thread":{"id":"55928","subject":"[PATCH 0/4] show-branch: add missing tests, trivial color output fix","startedAt":"2021-06-14T17:18:21Z","lastAt":"2021-06-17T21:46:59Z","messageCount":17,"participants":["Ævar Arnfjörð Bjarmason","Junio C Hamano","Michael J Gruber","Felipe Contreras"],"isPatch":true,"patchVersion":1,"patchTotal":4},"messages":[{"id":"427366","messageId":"cover-0.4-00000000000-20210614T171626Z-avarab@gmail.com","threadId":"55928","inReplyTo":null,"subject":"[PATCH 0/4] show-branch: add missing tests, trivial color output fix","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-06-14T17:18:06Z","receivedAt":"2021-06-14T17:18:21Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"This is a series I've had lying around for a while, when looking at\nthe color output for show-branch I noticed it reset and re-enabled\ncolor for every single space character.\n\nThis fixes that, but mostly fixes the mostly non-existing tests for\nthat old command it. It still has big blind spots, but now we have\nfewer blind spots.\n\nÆvar Arnfjörð Bjarmason (4):\n show-branch tests: rename the one \"show-branch\" test file\n show-branch tests: modernize test code\n show-branch: fix and test --color output\n show-branch tests: add missing tests\n\n builtin/show-branch.c | 9 +-\n t/t3202-show-branch-octopus.sh | 70 ----------------\n t/t3202-show-branch.sh | 149 +++++++++++++++++++++++++++++++++\n 3 files changed, 155 insertions(+), 73 deletions(-)\n delete mode 100755 t/t3202-show-branch-octopus.sh\n create mode 100755 t/t3202-show-branch.sh\n\n-- \n2.32.0.555.g0268d380f7b\n\n"},{"id":"427367","messageId":"patch-3.4-8db7029086d-20210614T171626Z-avarab@gmail.com","threadId":"55928","inReplyTo":"cover-0.4-00000000000-20210614T171626Z-avarab@gmail.com","subject":"[PATCH 3/4] show-branch: fix and test --color output","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-06-14T17:18:09Z","receivedAt":"2021-06-14T17:18:25Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Fix the \"show-branch --color\" output so it doesn't needlessly color\nand reset each time it emits a space character.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n builtin/show-branch.c | 9 ++++++---\n t/t3202-show-branch.sh | 30 ++++++++++++++++++++++++++++++\n 2 files changed, 36 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin/show-branch.c b/builtin/show-branch.c\nindex d6d2dabeca8..d77ce7aeb38 100644\n--- a/builtin/show-branch.c\n+++ b/builtin/show-branch.c\n@@ -939,9 +939,12 @@ int cmd_show_branch(int ac, const char **av, const char *prefix)\n \t\t\t\t\tmark = '*';\n \t\t\t\telse\n \t\t\t\t\tmark = '+';\n-\t\t\t\tprintf(\"%s%c%s\",\n-\t\t\t\t get_color_code(i),\n-\t\t\t\t mark, get_color_reset_code());\n+\t\t\t\tif (mark == ' ')\n+\t\t\t\t\tputchar(mark);\n+\t\t\t\telse\n+\t\t\t\t\tprintf(\"%s%c%s\",\n+\t\t\t\t\t get_color_code(i),\n+\t\t\t\t\t mark, get_color_reset_code());\n \t\t\t}\n \t\t\tputchar(' ');\n \t\t}\ndiff --git a/t/t3202-show-branch.sh b/t/t3202-show-branch.sh\nindex 7b06048905a..54025f03379 100755\n--- a/t/t3202-show-branch.sh\n+++ b/t/t3202-show-branch.sh\n@@ -55,4 +55,34 @@ test_expect_success 'show-branch with showbranch.default' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'show-branch --color output' '\n+\tsed \"s/^> //\" >expect <<-\\EOF &&\n+\t> ! [branch1] branch1\n+\t> ! [branch2] branch2\n+\t> ! [branch3] branch3\n+\t> ! [branch4] branch4\n+\t> ! [branch5] branch5\n+\t> ! [branch6] branch6\n+\t> ! [branch7] branch7\n+\t> ! [branch8] branch8\n+\t> ! [branch9] branch9\n+\t> * [branch10] branch10\n+\t> ----------\n+\t> * [branch10] branch10\n+\t> + [branch9] branch9\n+\t> + [branch8] branch8\n+\t> + [branch7] branch7\n+\t> + [branch6] branch6\n+\t> + [branch5] branch5\n+\t> + [branch4] branch4\n+\t> + [branch3] branch3\n+\t> + [branch2] branch2\n+\t> + [branch1] branch1\n+\t> +++++++++* [branch10^] initial\n+\tEOF\n+\tgit show-branch --color=always $(cat branches.sorted) >actual.raw &&\n+\ttest_decode_color actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_done\n-- \n2.32.0.555.g0268d380f7b\n\n"},{"id":"427368","messageId":"patch-4.4-c54c6a7b205-20210614T171626Z-avarab@gmail.com","threadId":"55928","inReplyTo":"cover-0.4-00000000000-20210614T171626Z-avarab@gmail.com","subject":"[PATCH 4/4] show-branch tests: add missing tests","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-06-14T17:18:10Z","receivedAt":"2021-06-14T17:18:28Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Add missing tests for --remotes, --list and --merge-base. These are\nnot exhaustive, but better than the nothing we have now.\n\nThere were some tests for this command added in f76412ed6db ([PATCH]\nAdd 'git show-branch'., 2005-08-21) has never been properly tested,\nnamely for the --all option in t6432-merge-recursive-space-options.sh,\nand some of --merge-base and --independent in t6010-merge-base.sh.\n\nThis fixes a few more blind spots, but there's still a lot of behavior\nthat's not tested for.\n\nThese new tests show the add (and possibly unintentional) behavior of\n--merge-base with one argument, and how its output is the same as \"git\nmerge-base\" with N bases in this particular case. See the test added\nin f621a8454d1 (git-merge-base/git-show-branch --merge-base:\nDocumentation and test, 2009-08-05) for a case where the two aren't\nthe same.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n t/t3202-show-branch.sh | 61 ++++++++++++++++++++++++++++++++++++++++++\n 1 file changed, 61 insertions(+)\n\ndiff --git a/t/t3202-show-branch.sh b/t/t3202-show-branch.sh\nindex 54025f03379..ad9902a06b9 100755\n--- a/t/t3202-show-branch.sh\n+++ b/t/t3202-show-branch.sh\n@@ -85,4 +85,65 @@ test_expect_success 'show-branch --color output' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'show branch --remotes' '\n+\tcat >expect.err <<-\\EOF &&\n+\tNo revs to be shown.\n+\tEOF\n+\tgit show-branch -r 2>actual.err >actual.out &&\n+\ttest_cmp expect.err actual.err &&\n+\ttest_must_be_empty actual.out\n+'\n+\n+test_expect_success 'setup show branch --list' '\n+\tsed \"s/^> //\" >expect <<-\\EOF\n+\t> [branch1] branch1\n+\t> [branch2] branch2\n+\t> [branch3] branch3\n+\t> [branch4] branch4\n+\t> [branch5] branch5\n+\t> [branch6] branch6\n+\t> [branch7] branch7\n+\t> [branch8] branch8\n+\t> [branch9] branch9\n+\t> * [branch10] branch10\n+\tEOF\n+'\n+\n+test_expect_success 'show branch --list' '\n+\tgit show-branch --list $(cat branches.sorted) >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'show branch --list has no --color output' '\n+\tgit show-branch --color=always --list $(cat branches.sorted) >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'show branch --merge-base with one argument' '\n+\tfor branch in $(cat branches.sorted)\n+\tdo\n+\t\tgit rev-parse $branch >expect &&\n+\t\tgit show-branch --merge-base $branch >actual &&\n+\t\ttest_cmp expect actual\n+\tdone\n+'\n+\n+test_expect_success 'show branch --merge-base with two arguments' '\n+\tfor branch in $(cat branches.sorted)\n+\tdo\n+\t\tgit rev-parse initial >expect &&\n+\t\tgit show-branch --merge-base initial $branch >actual &&\n+\t\ttest_cmp expect actual\n+\tdone\n+'\n+\n+test_expect_success 'show branch --merge-base with N arguments' '\n+\tgit rev-parse initial >expect &&\n+\tgit show-branch --merge-base $(cat branches.sorted) >actual &&\n+\ttest_cmp expect actual &&\n+\n+\tgit merge-base $(cat branches.sorted) >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_done\n-- \n2.32.0.555.g0268d380f7b\n\n"},{"id":"427369","messageId":"patch-1.4-7b8ac433392-20210614T171626Z-avarab@gmail.com","threadId":"55928","inReplyTo":"cover-0.4-00000000000-20210614T171626Z-avarab@gmail.com","subject":"[PATCH 1/4] show-branch tests: rename the one \"show-branch\" test file","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-06-14T17:18:07Z","receivedAt":"2021-06-14T17:19:18Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Rename the only *show-branch* test file to indicate that more tests\nbelong it in than just the one-off octopus test it now contains.\n\nThe test was initially added in ce567d1867a (Add test to show that\nshow-branch misses out the 8th column, 2008-07-23) and\n11ee57bc4c4 (sort_in_topological_order(): avoid setting a commit flag,\n2008-07-23). Those two add almost the same content, one with a\ntest_expect_success and the other a test_expect_failure (a bug being\ntested for was fixed on one of the branches).\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n t/{t3202-show-branch-octopus.sh => t3202-show-branch.sh} | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n rename t/{t3202-show-branch-octopus.sh => t3202-show-branch.sh} (95%)\n\ndiff --git a/t/t3202-show-branch-octopus.sh b/t/t3202-show-branch.sh\nsimilarity index 95%\nrename from t/t3202-show-branch-octopus.sh\nrename to t/t3202-show-branch.sh\nindex 5cb0126cfed..8cfbbf79c1b 100755\n--- a/t/t3202-show-branch-octopus.sh\n+++ b/t/t3202-show-branch.sh\n@@ -1,6 +1,6 @@\n #!/bin/sh\n \n-test_description='test show-branch with more than 8 heads'\n+test_description='test show-branch'\n \n GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main\n export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME\n-- \n2.32.0.555.g0268d380f7b\n\n"},{"id":"427370","messageId":"patch-2.4-27f94abaede-20210614T171626Z-avarab@gmail.com","threadId":"55928","inReplyTo":"cover-0.4-00000000000-20210614T171626Z-avarab@gmail.com","subject":"[PATCH 2/4] show-branch tests: modernize test code","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-06-14T17:18:08Z","receivedAt":"2021-06-14T17:19:22Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Modernize test code added in ce567d1867a (Add test to show that\nshow-branch misses out the 8th column, 2008-07-23) and\n11ee57bc4c4 (sort_in_topological_order(): avoid setting a commit flag,\n2008-07-23) to use test helpers.\n\nI'm renaming \"out\" to \"actual\" for consistency with other tests, and\nintroducing a \"branches.sorted\" file in the setup, to make it clear\nthat it's important that the list be sorted in this particular way.\n\nThe \"show-branch\" output is indented with spaces, which would cause\ncomplaints under \"git show --check\" with an indented here-doc\nblock. Let's prefix the lines with \"> \" to work around that, and to\nmake it clear that the leading whitespace is important.\n\nWe can also get rid of the hardcoding of \"main\" added here in\n334afbc76fb (tests: mark tests relying on the current default for\n`init.defaultBranch`, 2020-11-18). For this test we're setting up an\n\"initial\" commit anyway, and now that we've moved over to test_commit\nwe can reference that instead.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n t/t3202-show-branch.sh | 92 ++++++++++++++++++------------------------\n 1 file changed, 40 insertions(+), 52 deletions(-)\n\ndiff --git a/t/t3202-show-branch.sh b/t/t3202-show-branch.sh\nindex 8cfbbf79c1b..7b06048905a 100755\n--- a/t/t3202-show-branch.sh\n+++ b/t/t3202-show-branch.sh\n@@ -2,69 +2,57 @@\n \n test_description='test show-branch'\n \n-GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main\n-export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME\n-\n . ./test-lib.sh\n \n-numbers=\"1 2 3 4 5 6 7 8 9 10\"\n-\n test_expect_success 'setup' '\n-\n-\t> file &&\n-\tgit add file &&\n-\ttest_tick &&\n-\tgit commit -m initial &&\n-\n-\tfor i in $numbers\n+\ttest_commit initial &&\n+\tfor i in $(test_seq 1 10)\n \tdo\n-\t\tgit checkout -b branch$i main &&\n-\t\t> file$i &&\n-\t\tgit add file$i &&\n-\t\ttest_tick &&\n-\t\tgit commit -m branch$i || return 1\n-\tdone\n-\n+\t\tgit checkout -b branch$i initial &&\n+\t\ttest_commit --no-tag branch$i\n+\tdone &&\n+\tgit for-each-ref \\\n+\t\t--sort=version:refname \\\n+\t\t--format=\"%(refname:strip=2)\" \\\n+\t\t\"refs/heads/branch*\" >branches.sorted &&\n+\tsed \"s/^> //\" >expect <<-\\EOF\n+\t> ! [branch1] branch1\n+\t> ! [branch2] branch2\n+\t> ! [branch3] branch3\n+\t> ! [branch4] branch4\n+\t> ! [branch5] branch5\n+\t> ! [branch6] branch6\n+\t> ! [branch7] branch7\n+\t> ! [branch8] branch8\n+\t> ! [branch9] branch9\n+\t> * [branch10] branch10\n+\t> ----------\n+\t> * [branch10] branch10\n+\t> + [branch9] branch9\n+\t> + [branch8] branch8\n+\t> + [branch7] branch7\n+\t> + [branch6] branch6\n+\t> + [branch5] branch5\n+\t> + [branch4] branch4\n+\t> + [branch3] branch3\n+\t> + [branch2] branch2\n+\t> + [branch1] branch1\n+\t> +++++++++* [branch10^] initial\n+\tEOF\n '\n \n-cat > expect << EOF\n-! [branch1] branch1\n- ! [branch2] branch2\n- ! [branch3] branch3\n- ! [branch4] branch4\n- ! [branch5] branch5\n- ! [branch6] branch6\n- ! [branch7] branch7\n- ! [branch8] branch8\n- ! [branch9] branch9\n- * [branch10] branch10\n-----------\n- * [branch10] branch10\n- + [branch9] branch9\n- + [branch8] branch8\n- + [branch7] branch7\n- + [branch6] branch6\n- + [branch5] branch5\n- + [branch4] branch4\n- + [branch3] branch3\n- + [branch2] branch2\n-+ [branch1] branch1\n-+++++++++* [branch10^] initial\n-EOF\n-\n test_expect_success 'show-branch with more than 8 branches' '\n-\n-\tgit show-branch $(for i in $numbers; do echo branch$i; done) > out &&\n-\ttest_cmp expect out\n-\n+\tgit show-branch $(cat branches.sorted) >actual &&\n+\ttest_cmp expect actual\n '\n \n test_expect_success 'show-branch with showbranch.default' '\n-\tfor i in $numbers; do\n-\t\tgit config --add showbranch.default branch$i\n+\tfor branch in $(cat branches.sorted)\n+\tdo\n+\t\ttest_config showbranch.default $branch --add\n \tdone &&\n-\tgit show-branch >out &&\n-\ttest_cmp expect out\n+\tgit show-branch >actual &&\n+\ttest_cmp expect actual\n '\n \n test_done\n-- \n2.32.0.555.g0268d380f7b\n\n"},{"id":"427403","messageId":"xmqq8s3byg7k.fsf@gitster.g","threadId":"55928","inReplyTo":"cover-0.4-00000000000-20210614T171626Z-avarab@gmail.com","subject":"Re: [PATCH 0/4] show-branch: add missing tests, trivial color output fix","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-06-15T03:11:43Z","receivedAt":"2021-06-15T03:11:48Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> This is a series I've had lying around for a while, when looking at\n> the color output for show-branch I noticed it reset and re-enabled\n> color for every single space character.\n>\n> This fixes that, but mostly fixes the mostly non-existing tests for\n> that old command it. It still has big blind spots, but now we have\n> fewer blind spots.\n\nI am surprised if there are still users of this command that I wrote\nonly for my own use and that I no longer use more than once every\ntwo months ;-)\n\nThanks. Will queue.\n\n>\n> Ævar Arnfjörð Bjarmason (4):\n> show-branch tests: rename the one \"show-branch\" test file\n> show-branch tests: modernize test code\n> show-branch: fix and test --color output\n> show-branch tests: add missing tests\n>\n> builtin/show-branch.c | 9 +-\n> t/t3202-show-branch-octopus.sh | 70 ----------------\n> t/t3202-show-branch.sh | 149 +++++++++++++++++++++++++++++++++\n> 3 files changed, 155 insertions(+), 73 deletions(-)\n> delete mode 100755 t/t3202-show-branch-octopus.sh\n> create mode 100755 t/t3202-show-branch.sh\n"},{"id":"427430","messageId":"162374905722.40525.516266574605586007.git@grubix.eu","threadId":"55928","inReplyTo":"patch-4.4-c54c6a7b205-20210614T171626Z-avarab@gmail.com","subject":"Re: [PATCH 4/4] show-branch tests: add missing tests","fromName":"Michael J Gruber","fromEmail":"git@grubix.eu","sentAt":"2021-06-15T09:24:17Z","receivedAt":"2021-06-15T09:34:04Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Ævar Arnfjörð Bjarmason venit, vidit, dixit 2021-06-14 19:18:10:\n> Add missing tests for --remotes, --list and --merge-base. These are\n> not exhaustive, but better than the nothing we have now.\n> \n> There were some tests for this command added in f76412ed6db ([PATCH]\n> Add 'git show-branch'., 2005-08-21) has never been properly tested,\n> namely for the --all option in t6432-merge-recursive-space-options.sh,\n> and some of --merge-base and --independent in t6010-merge-base.sh.\n> \n> This fixes a few more blind spots, but there's still a lot of behavior\n> that's not tested for.\n> \n> These new tests show the add (and possibly unintentional) behavior of\n\n\"odd\"\n\nOther than that, I don't think show-branch was broken, so I somehow\ncontest the phrase \"fix\" in this series, it's more of a clean-up.\n\nJust to be sure: Users have no way of assigning a color code with\nbackground colors to the columns, which is why omitting\nthe lookup and reset is correct for a space.\n\nNow, people scripting around show-branch might be bitten by that change\nbecause the number of (unprocessed) characters in the output changes, or\nbecause a control character which they used to \"tr\" is not there any more.\nThey should not (script this command), I guess.\n\n> --merge-base with one argument, and how its output is the same as \"git\n> merge-base\" with N bases in this particular case. See the test added\n> in f621a8454d1 (git-merge-base/git-show-branch --merge-base:\n> Documentation and test, 2009-08-05) for a case where the two aren't\n> the same.\n> \n> Signed-off-by: Ævar Arnfjörð Bjarmason \n> ---\n> t/t3202-show-branch.sh | 61 ++++++++++++++++++++++++++++++++++++++++++\n> 1 file changed, 61 insertions(+)\n> \n> diff --git a/t/t3202-show-branch.sh b/t/t3202-show-branch.sh\n> index 54025f03379..ad9902a06b9 100755\n> --- a/t/t3202-show-branch.sh\n> +++ b/t/t3202-show-branch.sh\n> @@ -85,4 +85,65 @@ test_expect_success 'show-branch --color output' '\n> test_cmp expect actual\n> '\n> \n> +test_expect_success 'show branch --remotes' '\n> + cat >expect.err <<-\\EOF &&\n> + No revs to be shown.\n> + EOF\n> + git show-branch -r 2>actual.err >actual.out &&\n> + test_cmp expect.err actual.err &&\n> + test_must_be_empty actual.out\n> +'\n> +\n> +test_expect_success 'setup show branch --list' '\n> + sed \"s/^> //\" >expect <<-\\EOF\n> + > [branch1] branch1\n> + > [branch2] branch2\n> + > [branch3] branch3\n> + > [branch4] branch4\n> + > [branch5] branch5\n> + > [branch6] branch6\n> + > [branch7] branch7\n> + > [branch8] branch8\n> + > [branch9] branch9\n> + > * [branch10] branch10\n> + EOF\n> +'\n> +\n> +test_expect_success 'show branch --list' '\n> + git show-branch --list $(cat branches.sorted) >actual &&\n> + test_cmp expect actual\n> +'\n> +\n> +test_expect_success 'show branch --list has no --color output' '\n> + git show-branch --color=always --list $(cat branches.sorted) >actual &&\n> + test_cmp expect actual\n> +'\n> +\n> +test_expect_success 'show branch --merge-base with one argument' '\n> + for branch in $(cat branches.sorted)\n> + do\n> + git rev-parse $branch >expect &&\n> + git show-branch --merge-base $branch >actual &&\n> + test_cmp expect actual\n> + done\n> +'\n> +\n> +test_expect_success 'show branch --merge-base with two arguments' '\n> + for branch in $(cat branches.sorted)\n> + do\n> + git rev-parse initial >expect &&\n> + git show-branch --merge-base initial $branch >actual &&\n> + test_cmp expect actual\n> + done\n> +'\n> +\n> +test_expect_success 'show branch --merge-base with N arguments' '\n> + git rev-parse initial >expect &&\n> + git show-branch --merge-base $(cat branches.sorted) >actual &&\n> + test_cmp expect actual &&\n> +\n> + git merge-base $(cat branches.sorted) >actual &&\n> + test_cmp expect actual\n> +'\n> +\n> test_done\n> -- \n> 2.32.0.555.g0268d380f7b\n>\n"},{"id":"427728","messageId":"cover-0.4-0000000000-20210617T105245Z-avarab@gmail.com","threadId":"55928","inReplyTo":"cover-0.4-00000000000-20210614T171626Z-avarab@gmail.com","subject":"[PATCH v2 0/4] show-branch: add missing tests, trivial color output change","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-06-17T10:53:34Z","receivedAt":"2021-06-17T10:53:47Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"This v2 doesn't change any of the code (see range-diff), but better\nexplains the change per Michael J Gruber's feedback in\nhttps://lore.kernel.org/git/162374905722.40525.516266574605586007.git@grubix.eu/\n\nThere's also a trivial grammar fix, s/add/odd/g.\n\nÆvar Arnfjörð Bjarmason (4):\n show-branch tests: rename the one \"show-branch\" test file\n show-branch tests: modernize test code\n show-branch: don't for space characters\n show-branch tests: add missing tests\n\n builtin/show-branch.c | 9 +-\n t/t3202-show-branch-octopus.sh | 70 ----------------\n t/t3202-show-branch.sh | 149 +++++++++++++++++++++++++++++++++\n 3 files changed, 155 insertions(+), 73 deletions(-)\n delete mode 100755 t/t3202-show-branch-octopus.sh\n create mode 100755 t/t3202-show-branch.sh\n\nRange-diff against v1:\n1: 7b8ac43339 = 1: 7b8ac43339 show-branch tests: rename the one \"show-branch\" test file\n2: 27f94abaed = 2: 27f94abaed show-branch tests: modernize test code\n3: 8db7029086 ! 3: 937e728f7f show-branch: fix and test --color output\n @@ Metadata\n Author: Ævar Arnfjörð Bjarmason \n \n ## Commit message ##\n - show-branch: fix and test --color output\n + show-branch: don't for space characters\n \n - Fix the \"show-branch --color\" output so it doesn't needlessly color\n - and reset each time it emits a space character.\n + Change the colored output introduced in ab07ba2a24 (show-branch: color\n + the commit status signs, 2009-04-22) to not color and reset each\n + individual space character we use for padding. The intent is to color\n + just the \"!\", \"+\" etc. characters.\n +\n + This makes the output easier to test, so let's do that now. The test\n + would be much more verbose without a color/reset for each space\n + character. Since the coloring cycles through colors we previously had\n + a \"rainbow of space characters\".\n +\n + In theory this breaks things for anyone who's relying on the exact\n + colored output of show-branch, in practice I'd think anyone parsing it\n + isn't actively turning on the colored output.\n \n Signed-off-by: Ævar Arnfjörð Bjarmason \n \n4: c54c6a7b20 ! 4: dde0177235 show-branch tests: add missing tests\n @@ Commit message\n This fixes a few more blind spots, but there's still a lot of behavior\n that's not tested for.\n \n - These new tests show the add (and possibly unintentional) behavior of\n + These new tests show the odd (and possibly unintentional) behavior of\n --merge-base with one argument, and how its output is the same as \"git\n merge-base\" with N bases in this particular case. See the test added\n in f621a8454d1 (git-merge-base/git-show-branch --merge-base:\n-- \n2.32.0.571.gdba276db2c\n\n"},{"id":"427729","messageId":"patch-1.4-7b8ac43339-20210617T105245Z-avarab@gmail.com","threadId":"55928","inReplyTo":"cover-0.4-0000000000-20210617T105245Z-avarab@gmail.com","subject":"[PATCH v2 1/4] show-branch tests: rename the one \"show-branch\" test file","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-06-17T10:53:35Z","receivedAt":"2021-06-17T10:53:48Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Rename the only *show-branch* test file to indicate that more tests\nbelong it in than just the one-off octopus test it now contains.\n\nThe test was initially added in ce567d1867a (Add test to show that\nshow-branch misses out the 8th column, 2008-07-23) and\n11ee57bc4c4 (sort_in_topological_order(): avoid setting a commit flag,\n2008-07-23). Those two add almost the same content, one with a\ntest_expect_success and the other a test_expect_failure (a bug being\ntested for was fixed on one of the branches).\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n t/{t3202-show-branch-octopus.sh => t3202-show-branch.sh} | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n rename t/{t3202-show-branch-octopus.sh => t3202-show-branch.sh} (95%)\n\ndiff --git a/t/t3202-show-branch-octopus.sh b/t/t3202-show-branch.sh\nsimilarity index 95%\nrename from t/t3202-show-branch-octopus.sh\nrename to t/t3202-show-branch.sh\nindex 5cb0126cfe..8cfbbf79c1 100755\n--- a/t/t3202-show-branch-octopus.sh\n+++ b/t/t3202-show-branch.sh\n@@ -1,6 +1,6 @@\n #!/bin/sh\n \n-test_description='test show-branch with more than 8 heads'\n+test_description='test show-branch'\n \n GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main\n export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME\n-- \n2.32.0.571.gdba276db2c\n\n"},{"id":"427730","messageId":"patch-2.4-27f94abaed-20210617T105245Z-avarab@gmail.com","threadId":"55928","inReplyTo":"cover-0.4-0000000000-20210617T105245Z-avarab@gmail.com","subject":"[PATCH v2 2/4] show-branch tests: modernize test code","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-06-17T10:53:36Z","receivedAt":"2021-06-17T10:53:49Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Modernize test code added in ce567d1867a (Add test to show that\nshow-branch misses out the 8th column, 2008-07-23) and\n11ee57bc4c4 (sort_in_topological_order(): avoid setting a commit flag,\n2008-07-23) to use test helpers.\n\nI'm renaming \"out\" to \"actual\" for consistency with other tests, and\nintroducing a \"branches.sorted\" file in the setup, to make it clear\nthat it's important that the list be sorted in this particular way.\n\nThe \"show-branch\" output is indented with spaces, which would cause\ncomplaints under \"git show --check\" with an indented here-doc\nblock. Let's prefix the lines with \"> \" to work around that, and to\nmake it clear that the leading whitespace is important.\n\nWe can also get rid of the hardcoding of \"main\" added here in\n334afbc76fb (tests: mark tests relying on the current default for\n`init.defaultBranch`, 2020-11-18). For this test we're setting up an\n\"initial\" commit anyway, and now that we've moved over to test_commit\nwe can reference that instead.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n t/t3202-show-branch.sh | 92 ++++++++++++++++++------------------------\n 1 file changed, 40 insertions(+), 52 deletions(-)\n\ndiff --git a/t/t3202-show-branch.sh b/t/t3202-show-branch.sh\nindex 8cfbbf79c1..7b06048905 100755\n--- a/t/t3202-show-branch.sh\n+++ b/t/t3202-show-branch.sh\n@@ -2,69 +2,57 @@\n \n test_description='test show-branch'\n \n-GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main\n-export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME\n-\n . ./test-lib.sh\n \n-numbers=\"1 2 3 4 5 6 7 8 9 10\"\n-\n test_expect_success 'setup' '\n-\n-\t> file &&\n-\tgit add file &&\n-\ttest_tick &&\n-\tgit commit -m initial &&\n-\n-\tfor i in $numbers\n+\ttest_commit initial &&\n+\tfor i in $(test_seq 1 10)\n \tdo\n-\t\tgit checkout -b branch$i main &&\n-\t\t> file$i &&\n-\t\tgit add file$i &&\n-\t\ttest_tick &&\n-\t\tgit commit -m branch$i || return 1\n-\tdone\n-\n+\t\tgit checkout -b branch$i initial &&\n+\t\ttest_commit --no-tag branch$i\n+\tdone &&\n+\tgit for-each-ref \\\n+\t\t--sort=version:refname \\\n+\t\t--format=\"%(refname:strip=2)\" \\\n+\t\t\"refs/heads/branch*\" >branches.sorted &&\n+\tsed \"s/^> //\" >expect <<-\\EOF\n+\t> ! [branch1] branch1\n+\t> ! [branch2] branch2\n+\t> ! [branch3] branch3\n+\t> ! [branch4] branch4\n+\t> ! [branch5] branch5\n+\t> ! [branch6] branch6\n+\t> ! [branch7] branch7\n+\t> ! [branch8] branch8\n+\t> ! [branch9] branch9\n+\t> * [branch10] branch10\n+\t> ----------\n+\t> * [branch10] branch10\n+\t> + [branch9] branch9\n+\t> + [branch8] branch8\n+\t> + [branch7] branch7\n+\t> + [branch6] branch6\n+\t> + [branch5] branch5\n+\t> + [branch4] branch4\n+\t> + [branch3] branch3\n+\t> + [branch2] branch2\n+\t> + [branch1] branch1\n+\t> +++++++++* [branch10^] initial\n+\tEOF\n '\n \n-cat > expect << EOF\n-! [branch1] branch1\n- ! [branch2] branch2\n- ! [branch3] branch3\n- ! [branch4] branch4\n- ! [branch5] branch5\n- ! [branch6] branch6\n- ! [branch7] branch7\n- ! [branch8] branch8\n- ! [branch9] branch9\n- * [branch10] branch10\n-----------\n- * [branch10] branch10\n- + [branch9] branch9\n- + [branch8] branch8\n- + [branch7] branch7\n- + [branch6] branch6\n- + [branch5] branch5\n- + [branch4] branch4\n- + [branch3] branch3\n- + [branch2] branch2\n-+ [branch1] branch1\n-+++++++++* [branch10^] initial\n-EOF\n-\n test_expect_success 'show-branch with more than 8 branches' '\n-\n-\tgit show-branch $(for i in $numbers; do echo branch$i; done) > out &&\n-\ttest_cmp expect out\n-\n+\tgit show-branch $(cat branches.sorted) >actual &&\n+\ttest_cmp expect actual\n '\n \n test_expect_success 'show-branch with showbranch.default' '\n-\tfor i in $numbers; do\n-\t\tgit config --add showbranch.default branch$i\n+\tfor branch in $(cat branches.sorted)\n+\tdo\n+\t\ttest_config showbranch.default $branch --add\n \tdone &&\n-\tgit show-branch >out &&\n-\ttest_cmp expect out\n+\tgit show-branch >actual &&\n+\ttest_cmp expect actual\n '\n \n test_done\n-- \n2.32.0.571.gdba276db2c\n\n"},{"id":"427731","messageId":"patch-4.4-dde0177235-20210617T105245Z-avarab@gmail.com","threadId":"55928","inReplyTo":"cover-0.4-0000000000-20210617T105245Z-avarab@gmail.com","subject":"[PATCH v2 4/4] show-branch tests: add missing tests","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-06-17T10:53:38Z","receivedAt":"2021-06-17T10:53:51Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Add missing tests for --remotes, --list and --merge-base. These are\nnot exhaustive, but better than the nothing we have now.\n\nThere were some tests for this command added in f76412ed6db ([PATCH]\nAdd 'git show-branch'., 2005-08-21) has never been properly tested,\nnamely for the --all option in t6432-merge-recursive-space-options.sh,\nand some of --merge-base and --independent in t6010-merge-base.sh.\n\nThis fixes a few more blind spots, but there's still a lot of behavior\nthat's not tested for.\n\nThese new tests show the odd (and possibly unintentional) behavior of\n--merge-base with one argument, and how its output is the same as \"git\nmerge-base\" with N bases in this particular case. See the test added\nin f621a8454d1 (git-merge-base/git-show-branch --merge-base:\nDocumentation and test, 2009-08-05) for a case where the two aren't\nthe same.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n t/t3202-show-branch.sh | 61 ++++++++++++++++++++++++++++++++++++++++++\n 1 file changed, 61 insertions(+)\n\ndiff --git a/t/t3202-show-branch.sh b/t/t3202-show-branch.sh\nindex 54025f0337..ad9902a06b 100755\n--- a/t/t3202-show-branch.sh\n+++ b/t/t3202-show-branch.sh\n@@ -85,4 +85,65 @@ test_expect_success 'show-branch --color output' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'show branch --remotes' '\n+\tcat >expect.err <<-\\EOF &&\n+\tNo revs to be shown.\n+\tEOF\n+\tgit show-branch -r 2>actual.err >actual.out &&\n+\ttest_cmp expect.err actual.err &&\n+\ttest_must_be_empty actual.out\n+'\n+\n+test_expect_success 'setup show branch --list' '\n+\tsed \"s/^> //\" >expect <<-\\EOF\n+\t> [branch1] branch1\n+\t> [branch2] branch2\n+\t> [branch3] branch3\n+\t> [branch4] branch4\n+\t> [branch5] branch5\n+\t> [branch6] branch6\n+\t> [branch7] branch7\n+\t> [branch8] branch8\n+\t> [branch9] branch9\n+\t> * [branch10] branch10\n+\tEOF\n+'\n+\n+test_expect_success 'show branch --list' '\n+\tgit show-branch --list $(cat branches.sorted) >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'show branch --list has no --color output' '\n+\tgit show-branch --color=always --list $(cat branches.sorted) >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'show branch --merge-base with one argument' '\n+\tfor branch in $(cat branches.sorted)\n+\tdo\n+\t\tgit rev-parse $branch >expect &&\n+\t\tgit show-branch --merge-base $branch >actual &&\n+\t\ttest_cmp expect actual\n+\tdone\n+'\n+\n+test_expect_success 'show branch --merge-base with two arguments' '\n+\tfor branch in $(cat branches.sorted)\n+\tdo\n+\t\tgit rev-parse initial >expect &&\n+\t\tgit show-branch --merge-base initial $branch >actual &&\n+\t\ttest_cmp expect actual\n+\tdone\n+'\n+\n+test_expect_success 'show branch --merge-base with N arguments' '\n+\tgit rev-parse initial >expect &&\n+\tgit show-branch --merge-base $(cat branches.sorted) >actual &&\n+\ttest_cmp expect actual &&\n+\n+\tgit merge-base $(cat branches.sorted) >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_done\n-- \n2.32.0.571.gdba276db2c\n\n"},{"id":"427732","messageId":"patch-3.4-937e728f7f-20210617T105245Z-avarab@gmail.com","threadId":"55928","inReplyTo":"cover-0.4-0000000000-20210617T105245Z-avarab@gmail.com","subject":"[PATCH v2 3/4] show-branch: don't for space characters","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-06-17T10:53:37Z","receivedAt":"2021-06-17T10:53:52Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Change the colored output introduced in ab07ba2a24 (show-branch: color\nthe commit status signs, 2009-04-22) to not color and reset each\nindividual space character we use for padding. The intent is to color\njust the \"!\", \"+\" etc. characters.\n\nThis makes the output easier to test, so let's do that now. The test\nwould be much more verbose without a color/reset for each space\ncharacter. Since the coloring cycles through colors we previously had\na \"rainbow of space characters\".\n\nIn theory this breaks things for anyone who's relying on the exact\ncolored output of show-branch, in practice I'd think anyone parsing it\nisn't actively turning on the colored output.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n builtin/show-branch.c | 9 ++++++---\n t/t3202-show-branch.sh | 30 ++++++++++++++++++++++++++++++\n 2 files changed, 36 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin/show-branch.c b/builtin/show-branch.c\nindex d6d2dabeca..d77ce7aeb3 100644\n--- a/builtin/show-branch.c\n+++ b/builtin/show-branch.c\n@@ -939,9 +939,12 @@ int cmd_show_branch(int ac, const char **av, const char *prefix)\n \t\t\t\t\tmark = '*';\n \t\t\t\telse\n \t\t\t\t\tmark = '+';\n-\t\t\t\tprintf(\"%s%c%s\",\n-\t\t\t\t get_color_code(i),\n-\t\t\t\t mark, get_color_reset_code());\n+\t\t\t\tif (mark == ' ')\n+\t\t\t\t\tputchar(mark);\n+\t\t\t\telse\n+\t\t\t\t\tprintf(\"%s%c%s\",\n+\t\t\t\t\t get_color_code(i),\n+\t\t\t\t\t mark, get_color_reset_code());\n \t\t\t}\n \t\t\tputchar(' ');\n \t\t}\ndiff --git a/t/t3202-show-branch.sh b/t/t3202-show-branch.sh\nindex 7b06048905..54025f0337 100755\n--- a/t/t3202-show-branch.sh\n+++ b/t/t3202-show-branch.sh\n@@ -55,4 +55,34 @@ test_expect_success 'show-branch with showbranch.default' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'show-branch --color output' '\n+\tsed \"s/^> //\" >expect <<-\\EOF &&\n+\t> ! [branch1] branch1\n+\t> ! [branch2] branch2\n+\t> ! [branch3] branch3\n+\t> ! [branch4] branch4\n+\t> ! [branch5] branch5\n+\t> ! [branch6] branch6\n+\t> ! [branch7] branch7\n+\t> ! [branch8] branch8\n+\t> ! [branch9] branch9\n+\t> * [branch10] branch10\n+\t> ----------\n+\t> * [branch10] branch10\n+\t> + [branch9] branch9\n+\t> + [branch8] branch8\n+\t> + [branch7] branch7\n+\t> + [branch6] branch6\n+\t> + [branch5] branch5\n+\t> + [branch4] branch4\n+\t> + [branch3] branch3\n+\t> + [branch2] branch2\n+\t> + [branch1] branch1\n+\t> +++++++++* [branch10^] initial\n+\tEOF\n+\tgit show-branch --color=always $(cat branches.sorted) >actual.raw &&\n+\ttest_decode_color actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_done\n-- \n2.32.0.571.gdba276db2c\n\n"},{"id":"427790","messageId":"60cbbb98c40b3_1299208ea@natae.notmuch","threadId":"55928","inReplyTo":"patch-1.4-7b8ac43339-20210617T105245Z-avarab@gmail.com","subject":"RE: [PATCH v2 1/4] show-branch tests: rename the one \"show-branch\" test file","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2021-06-17T21:16:08Z","receivedAt":"2021-06-17T21:16:13Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Ævar Arnfjörð Bjarmason wrote:\n> diff --git a/t/t3202-show-branch-octopus.sh b/t/t3202-show-branch.sh\n> similarity index 95%\n> rename from t/t3202-show-branch-octopus.sh\n> rename to t/t3202-show-branch.sh\n> index 5cb0126cfe..8cfbbf79c1 100755\n> --- a/t/t3202-show-branch-octopus.sh\n> +++ b/t/t3202-show-branch.sh\n\nMakes sense.\n\n-- \nFelipe Contreras"},{"id":"427791","messageId":"60cbbe9d89cbd_1299208fa@natae.notmuch","threadId":"55928","inReplyTo":"patch-2.4-27f94abaed-20210617T105245Z-avarab@gmail.com","subject":"RE: [PATCH v2 2/4] show-branch tests: modernize test code","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2021-06-17T21:29:01Z","receivedAt":"2021-06-17T21:29:06Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Ævar Arnfjörð Bjarmason wrote:\n> Modernize test code added in ce567d1867a (Add test to show that\n> show-branch misses out the 8th column, 2008-07-23) and\n> 11ee57bc4c4 (sort_in_topological_order(): avoid setting a commit flag,\n> 2008-07-23) to use test helpers.\n> \n> I'm renaming \"out\" to \"actual\" for consistency with other tests, and\n> introducing a \"branches.sorted\" file in the setup, to make it clear\n> that it's important that the list be sorted in this particular way.\n\nThat is better.\n\n> The \"show-branch\" output is indented with spaces, which would cause\n> complaints under \"git show --check\" with an indented here-doc\n> block. Let's prefix the lines with \"> \" to work around that, and to\n> make it clear that the leading whitespace is important.\n\nI'm not sure this is an improvement. To me the original code is just\nfine. Also, I don't think writing an 'expect' file belong in a setup\nstep.\n\nAdditionally I would do this change in a separate patch.\n\n> We can also get rid of the hardcoding of \"main\" added here in\n> 334afbc76fb (tests: mark tests relying on the current default for\n> `init.defaultBranch`, 2020-11-18). For this test we're setting up an\n> \"initial\" commit anyway, and now that we've moved over to test_commit\n> we can reference that instead.\n\nThat's also good.\n\nAll the changes in this patch look good to me, however, they are smashed\ntogether in a way that makes the review harder, I see:\n\n 1. Use test_commit\n 2. Rename out to actual\n 3. Use >actual instead of > actual\n 4. Use test_seq instead of $numbers\n 5. Use branches.sorted instead of branch$i\n 6. Use test_config instead of git config\n 7. Use internal sed 's/^ //' instead of outside cat\n\nI'm on-board with 6 out of 7, but if these were done it at least 2\npatches, they would be clearer. I in fact would prefer one patch per\nchange (although maybe squash 3 with 2).\n\nCheers.\n\n-- \nFelipe Contreras"},{"id":"427792","messageId":"60cbc198d0552_1299208e8@natae.notmuch","threadId":"55928","inReplyTo":"patch-3.4-937e728f7f-20210617T105245Z-avarab@gmail.com","subject":"RE: [PATCH v2 3/4] show-branch: don't for space characters","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2021-06-17T21:41:44Z","receivedAt":"2021-06-17T21:41:48Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Ævar Arnfjörð Bjarmason wrote:\n> Change the colored output introduced in ab07ba2a24 (show-branch: color\n> the commit status signs, 2009-04-22) to not color and reset each\n> individual space character we use for padding. The intent is to color\n> just the \"!\", \"+\" etc. characters.\n\nObviously a fix, but perhaps show the current behavior:\n\n+ [branch1] branch1\n\nVersus:\n\n+ [branch1] branch1\n\n> In theory this breaks things for anyone who's relying on the exact\n> colored output of show-branch, in practice I'd think anyone parsing it\n> isn't actively turning on the colored output.\n\nPlease let's limit our worries to real users. Few people use\n`git show-branch`, even less would rely on parsing its bogus output.\n\n> --- a/builtin/show-branch.c\n> +++ b/builtin/show-branch.c\n> @@ -939,9 +939,12 @@ int cmd_show_branch(int ac, const char **av, const char *prefix)\n> \t\t\t\t\tmark = '*';\n> \t\t\t\telse\n> \t\t\t\t\tmark = '+';\n> -\t\t\t\tprintf(\"%s%c%s\",\n> -\t\t\t\t get_color_code(i),\n> -\t\t\t\t mark, get_color_reset_code());\n> +\t\t\t\tif (mark == ' ')\n> +\t\t\t\t\tputchar(mark);\n> +\t\t\t\telse\n> +\t\t\t\t\tprintf(\"%s%c%s\",\n> +\t\t\t\t\t get_color_code(i),\n> +\t\t\t\t\t mark, get_color_reset_code());\n\nVery simple and obvious improvement.\n\nCheers.\n\n-- \nFelipe Contreras"},{"id":"427793","messageId":"60cbc22e3db95_129920882@natae.notmuch","threadId":"55928","inReplyTo":"patch-4.4-dde0177235-20210617T105245Z-avarab@gmail.com","subject":"RE: [PATCH v2 4/4] show-branch tests: add missing tests","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2021-06-17T21:44:14Z","receivedAt":"2021-06-17T21:44:18Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Ævar Arnfjörð Bjarmason wrote:\n> Add missing tests for --remotes, --list and --merge-base. These are\n> not exhaustive, but better than the nothing we have now.\n\nIndeed, this is better.\n\n> --- a/t/t3202-show-branch.sh\n> +++ b/t/t3202-show-branch.sh\n> @@ -85,4 +85,65 @@ test_expect_success 'show-branch --color output' '\n> \ttest_cmp expect actual\n> '\n> \n> +test_expect_success 'show branch --remotes' '\n> +\tcat >expect.err <<-\\EOF &&\n> +\tNo revs to be shown.\n> +\tEOF\n> +\tgit show-branch -r 2>actual.err >actual.out &&\n> +\ttest_cmp expect.err actual.err &&\n> +\ttest_must_be_empty actual.out\n> +'\n> +\n> +test_expect_success 'setup show branch --list' '\n> +\tsed \"s/^> //\" >expect <<-\\EOF\n> +\t> [branch1] branch1\n> +\t> [branch2] branch2\n> +\t> [branch3] branch3\n> +\t> [branch4] branch4\n> +\t> [branch5] branch5\n> +\t> [branch6] branch6\n> +\t> [branch7] branch7\n> +\t> [branch8] branch8\n> +\t> [branch9] branch9\n> +\t> * [branch10] branch10\n> +\tEOF\n> +'\n> +\n> +test_expect_success 'show branch --list' '\n> +\tgit show-branch --list $(cat branches.sorted) >actual &&\n> +\ttest_cmp expect actual\n> +'\n> +\n> +test_expect_success 'show branch --list has no --color output' '\n> +\tgit show-branch --color=always --list $(cat branches.sorted) >actual &&\n> +\ttest_cmp expect actual\n> +'\n> +\n> +test_expect_success 'show branch --merge-base with one argument' '\n> +\tfor branch in $(cat branches.sorted)\n> +\tdo\n> +\t\tgit rev-parse $branch >expect &&\n> +\t\tgit show-branch --merge-base $branch >actual &&\n> +\t\ttest_cmp expect actual\n> +\tdone\n> +'\n> +\n> +test_expect_success 'show branch --merge-base with two arguments' '\n> +\tfor branch in $(cat branches.sorted)\n> +\tdo\n> +\t\tgit rev-parse initial >expect &&\n> +\t\tgit show-branch --merge-base initial $branch >actual &&\n> +\t\ttest_cmp expect actual\n> +\tdone\n> +'\n> +\n> +test_expect_success 'show branch --merge-base with N arguments' '\n> +\tgit rev-parse initial >expect &&\n> +\tgit show-branch --merge-base $(cat branches.sorted) >actual &&\n> +\ttest_cmp expect actual &&\n> +\n> +\tgit merge-base $(cat branches.sorted) >actual &&\n> +\ttest_cmp expect actual\n> +'\n> +\n> test_done\n\nAll these look good to me.\n\nCheers.\n\n-- \nFelipe Contreras"},{"id":"427794","messageId":"60cbc2cfde385_12992083@natae.notmuch","threadId":"55928","inReplyTo":"cover-0.4-0000000000-20210617T105245Z-avarab@gmail.com","subject":"RE: [PATCH v2 0/4] show-branch: add missing tests, trivial color output change","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2021-06-17T21:46:55Z","receivedAt":"2021-06-17T21:46:59Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Ævar Arnfjörð Bjarmason wrote:\n> This v2 doesn't change any of the code (see range-diff), but better\n> explains the change per Michael J Gruber's feedback in\n> https://lore.kernel.org/git/162374905722.40525.516266574605586007.git@grubix.eu/\n> \n> There's also a trivial grammar fix, s/add/odd/g.\n> \n> Ævar Arnfjörð Bjarmason (4):\n> show-branch tests: rename the one \"show-branch\" test file\n> show-branch tests: modernize test code\n> show-branch: don't for space characters\n> show-branch tests: add missing tests\n\nAll these look good to me (I would prefer patch #2 to be split into\nmultiple patches, but that's not a deal-breaker).\n\nReviewed-by: Felipe Contreras \n\n-- \nFelipe Contreras"}]}