Re: [PATCH 2/2] ref-filter: name the worktree of a branch being rebased or bisected

[Jasper Mayone <me@jaspermayone.com>]

Phillip Wood <phillip.wood123@gmail.com> writes:

> Those commands also reject branches that are being updated by a rebase
> via "update-ref" commands. If we're interested in checkouts we don't
> want to list those, but it means that "%(worktree)" will still give
> different results.

Yes, that difference is on purpose. I will say so in the commit message
and the documentation.

> I don't really follow this logic. At the moment neither "git
> for-each-ref", nor "git worktree list" show this - there is no way for
> a script to find out which worktree is working on a given branch. We
> can choose to add it to one or both commands.

You are right, that paragraph is wrong. No command shows it today, and
this patch adds it to %(worktreepath) only. I will rewrite it.

> If we want it to behave like prepare_checked_out_branches() we should
> use that function, rather than copying some of it to a separate file.

Agreed. For v4 I have added a small function to branch.c,

    const char *branch_checked_out_worktree(const char *refname);

which reads the list that prepare_checked_out_branches() builds and
skips the BRANCH_CHECKOUT_KIND_UPDATE_REF entries. ref-filter calls it
and drops its own hashmap, so the patch now removes more code than it
adds in ref-filter.c. t6302, t3203, t3200, t2400 and t2403 pass with it.

> That sounds a bit confusing - "* (no branch, rebasing topic)" already
> tells us what we need to know, why do we need "+ topic" as well?

I agree. In v4 "git branch" will not mark the branch with "+" in the
worktree that is rebasing or bisecting it, since the "*" line already
names it. The "also in the worktree" sentence in git-branch.adoc goes
away with that.

> It is not a new problem, but I think it would be helpful to add
> something like
>
>     As the path may contain a newline, [...]
>
>     Note that if a branch checked out in more than one worktree,
>     `%(worktreepath)` will only show one of those worktrees.

Thanks, I will add that as a separate documentation patch before this
one.

>> +		if (wt->is_bare)
>> +			continue;
>
> This is a change in behavior. We currently print the git_dir of bare
> worktrees. I think you can argue that this change is a bug fix as the
> user asked for the worktree path, but it should be explained in the
> commit message.

Agreed. With a bare repository and a linked worktree, today "git
branch" in the linked worktree shows "+ main" for the bare
repository's HEAD, and %(worktreepath) gives the path of the bare
repository. main is not checked out anywhere, and "git worktree add"
already lets you check it out. I will explain that in the commit
message and add a test for it.

Thanks,
Jasper