On Tue, Feb 19, 2019 at 05:31:22PM +0900, [email protected] wrote:
> From: Nickolai Belakovski <[email protected]>
>
> The output of git branch is modified to mark branches checkout out in a
s/checkout out/checked out/ ?
> linked worktree with a "+" and color them in cyan (in contrast to the
> current branch, which will still be denoted with a "*" and colored in green)
>
> This is meant to communicate to the user that the branches that are
> marked or colored will behave differently from other branches if the user
> attempts to check them out or delete them, since branches checked out in
> another worktree cannot be checked out or deleted.
I think this makes sense to have. You cannot "git checkout" such a
marked branch (since it would conflict with the other worktree), and
that alone seems to make it worth marking in the output.
> diff --git a/Documentation/git-branch.txt b/Documentation/git-branch.txt
> index 3bd83a7cbd..f2e5a07d64 100644
> --- a/Documentation/git-branch.txt
> +++ b/Documentation/git-branch.txt
> @@ -26,8 +26,10 @@ DESCRIPTION
> -----------
>
> If `--list` is given, or if there are no non-option arguments, existing
> -branches are listed; the current branch will be highlighted with an
> -asterisk. Option `-r` causes the remote-tracking branches to be listed,
> +branches are listed; the current branch will be highlighted in green and
> +marked with an asterisk. Any branches checked out in linked worktrees will
> +be highlighted in cyan and marked with a plus sign. Option `-r` causes the
> +remote-tracking branches to be listed,
This makes sense to me.
> - strbuf_addf(&local, "%%(if)%%(HEAD)%%(then)* %s%%(else) %s%%(end)",
> - branch_get_color(BRANCH_COLOR_CURRENT),
> - branch_get_color(BRANCH_COLOR_LOCAL));
> + strbuf_addf(&local, "%%(if)%%(HEAD)%%(then)*
> %s%%(else)%%(if)%%(worktreepath)%%(then)+ %s%%(else) %s%%(end)%%(end)",
> + branch_get_color(BRANCH_COLOR_CURRENT),
> + branch_get_color(BRANCH_COLOR_WORKTREE),
> + branch_get_color(BRANCH_COLOR_LOCAL));
Makes sense. The long line is ugly. Our format does not support breaking
long lines, though we could break the C string, I think, like:
strbuf_add(&local,
"%%(if)%%(HEAD)"
"%%(then)* %s"
"%%(else)%(if)%%(worktreepath)"
"%%(then)+ %s"
"%%(else)"
"%%(then) %s"
"%%(end)%%(end)");
That's pretty ugly, too, but it at least shows the conditional
structure.
> diff --git a/t/t3203-branch-output.sh b/t/t3203-branch-output.sh
> index ee6787614c..94ab05ad59 100755
> --- a/t/t3203-branch-output.sh
> +++ b/t/t3203-branch-output.sh
> @@ -240,6 +240,27 @@ test_expect_success 'git branch --format option' '
> test_i18ncmp expect actual
> '
>
> +test_expect_success '"add" a worktree' '
> + mkdir worktree_dir &&
> + git worktree add -b master_worktree worktree_dir master
> +'
This mkdir gets deleted in the next patch. It should just not be added
here.
> +cat >expect <<'EOF'
> +* <GREEN>(HEAD detached from fromtag)<RESET>
> + ambiguous<RESET>
> + branch-one<RESET>
> + branch-two<RESET>
> + master<RESET>
> ++ <CYAN>master_worktree<RESET>
> + ref-to-branch<RESET> -> branch-one
> + ref-to-remote<RESET> -> origin/branch-one
> +EOF
> +test_expect_success TTY 'worktree colors correct' '
> + test_terminal git branch >actual.raw &&
> + test_decode_color <actual.raw >actual &&
> + test_cmp expect actual
> +'
We are not testing the auto-color behavior here, so I think you could
just use "git branch --color >actual.raw" and drop the TTY prerequisite.
That's shorter and simpler, and will let your test run on more
platforms.
-Peff
