On Thu, Feb 21, 2019 at 4:59 AM Jeff King <[email protected]> wrote:
>
> On Tue, Feb 19, 2019 at 05:31:23PM +0900, [email protected] wrote:
>
> > From: Nickolai Belakovski <[email protected]>
> >
> > To display worktree path for refs checked out in a linked worktree
>
> This would be a good place to describe why this is useful. :)
>
> I do not have an opinion myself. Patch 2 makes a lot of sense to me, but
> I don't know if people would like this one or not. I don't use "-v"
> myself, though, so what do I know. :)
I threw this one in because I thought it wouldn't be clear to the
average user why some
branches are in cyan. By putting the worktree path in cyan on the next
level of output
I thought this would help the user make the connection, but actually I
don't have strong
feelings about this one.
>
> > diff --git a/Documentation/git-branch.txt b/Documentation/git-branch.txt
> > index f2e5a07d64..326a45f648 100644
> > --- a/Documentation/git-branch.txt
> > +++ b/Documentation/git-branch.txt
> > @@ -168,8 +168,10 @@ This option is only applicable in non-verbose mode.
> > When in list mode,
> > show sha1 and commit subject line for each head, along with
> > relationship to upstream branch (if any). If given twice, print
> > - the name of the upstream branch, as well (see also `git remote
> > - show <remote>`).
> > + the path of the linked worktree, if applicable (not applicable
> > + for current worktree since user's path will already be in current
> > + worktree) and the name of the upstream branch, as well (see also
> > + `git remote show <remote>`).
>
> That parenthetical feels a bit awkward. Maybe:
>
> ...print the path of the linked worktree (if any) and the name of the
> upstream branch, as well (see also `git remote show <remote>`). Note
> that the current worktree's HEAD will not have its path printed (it
> will always be your current directory).
Sure I can make that change
>
> > diff --git a/builtin/branch.c b/builtin/branch.c
> > index c2a86362bb..0b8ba9e4c5 100644
> > --- a/builtin/branch.c
> > +++ b/builtin/branch.c
> > @@ -367,9 +367,13 @@ static char *build_format(struct ref_filter *filter,
> > int maxwidth, const char *r
> > strbuf_addf(&local, " %s ", obname.buf);
> >
> > if (filter->verbose > 1)
> > + {
> > + strbuf_addf(&local,
> > "%%(if:notequals=*)%%(HEAD)%%(then)%%(if)%%(worktreepath)%%(then)(%s%%(worktreepath)%s)
> > %%(end)%%(end)",
> > + branch_get_color(BRANCH_COLOR_WORKTREE),
> > branch_get_color(BRANCH_COLOR_RESET));
> > strbuf_addf(&local,
> > "%%(if)%%(upstream)%%(then)[%s%%(upstream:short)%s%%(if)%%(upstream:track)"
> > "%%(then):
> > %%(upstream:track,nobracket)%%(end)] %%(end)%%(contents:subject)",
> > branch_get_color(BRANCH_COLOR_UPSTREAM),
> > branch_get_color(BRANCH_COLOR_RESET));
> > + }
>
> Another unreadable long line (both the one you're adding, and the existing
> one!). I don't know if it's worth trying to clean these up, but if we
> do, it might be worth hitting the existing ones, too.
>
> I'm OK if that comes as a patch on top later on, though.
Agreed, but there's enough lines like this that it'll just look
inconsistent if only one were broken up.
>
>
> diff --git a/t/t3203-branch-output.sh b/t/t3203-branch-output.sh
> index 012ddde7f2..8065279be6 100755
> --- a/t/t3203-branch-output.sh
> +++ b/t/t3203-branch-output.sh
> @@ -284,22 +284,20 @@ test_expect_success '--color overrides auto-color' '
> test_cmp expect.color actual
> '
>
> -# This test case has some special code to strip the first 30 characters or so
> -# of the output so that we do not have to put commit hashes into the expect
> test_expect_success 'verbose output lists worktree path' '
> + one=$(git rev-parse --short HEAD) &&
> + two=$(git rev-parse --short master) &&
> cat >expect <<-EOF &&
> - one
> - one
> - two
> - one
> - two
> - ($(pwd)/worktree_dir) two
> - two
> - two
> + * (HEAD detached from fromtag) $one one
> + ambiguous $one one
> + branch-one $two two
> + branch-two $one one
> + master $two two
> + + master_worktree $two ($(pwd)/worktree_dir) two
> + ref-to-branch $two two
> + ref-to-remote $two two
> EOF
> - git branch -vv >tmp &&
> - SUBSTRLENGTH=$(head -1 tmp | awk "{print index(\$0, \"one\")}") &&
> - awk -v substrlength="$SUBSTRLENGTH" "{print
> substr(\$0,substrlength,length(\$0))}" <tmp >actual &&
> + git branch -vv >actual &&
> test_cmp expect actual
> '
>
>
> I don't like how it depends on the space alignment of the branches, but
> I do like that you can clearly see which branch is being annotated.
Thanks for the suggestion. While I'm kinda proud of my awk thing, I
think yours is a lot easier to read. Will add.
>
> -Peff
