Re: [PATCH v3] worktree add: improve message for ambiguous remote branch name

Junio C Hamano <[email protected]>
Newsgroups org.kernel.vger.git
Message-ID <[email protected]>
"Yoichi NAKAYAMA via GitGitGadget" <[email protected]> writes:

> From: Yoichi NAKAYAMA <[email protected]>
>
> When the user runs 'git worktree add x y' command that does not
> exactly say which remote they want to work with, and there is no local
> branch named y, we try to guess which remote by passing y then create
> a new branch named y which tracks the remote branch.

I used x and y as placeholders.  The readers would be helped if you
used a more plausible sounding names, e.g., naming directory as
something like foo-dir (the point being 'dir' somewhere in its name)
and naming a branch as something like bar-topic.  If this were 'git
worktree add', it is probably more than likely that the destination
directory would begin with ../ to have the new worktree next to the
primary repository we are running in, no?

> If there are multiple remotes that have branch named y, we silently
> gave up, leaving the variable branch intact.  This later causes
> creating local branch and worktree not happen, and we end up with
> passing an non-existing branch to lookup_commit_reference_by_name(),
> triggering "invalid reference" error and die.

"This later causes" part still seems a bit too sketchy to help a
totally new reader, even though I've stared at this code long enough
so it would be sufficient for me personally.  But these logs are not
about helping me, but helping other developers, so...

> +#define WORKTREE_ADD_AMBIGUOUS_REMOTE_BRANCH_NAME_HINT_TEXT \
> +	_("Matched multiple remote tracking branches, you can list them by:\n" \
> +	"\n" \
> +	"    git branch -r --list \"*/%s\"\n" \
> +	"\n" \
> +	"If you meant to create a worktree from a remote tracking branch on,\n" \
> +	"e.g. 'origin', you can do so by:\n" \
> +	"\n" \
> +	"    git worktree add -b %s %s origin/%s\n" \
> +	"\n" \
> +	"If you'd like to always prefer some remote, e.g. 'origin',\n" \
> +	"consider setting checkout.defaultRemote=origin in your config.")

Instead of throwing the problem back to the user with four extra
lines of message telling them how to run 'git branch', I would have
expected this patch to teach unique_tracking_name() to optionally
return the list of remotes with that branch name, and to use that
result in this message.  However, if the goal is simply to provide
something better than 'invalid reference', we do not even need to
go that far.  Just stating that branch 'y' appears on multiple
remotes and asking them to clarify which one they mean might be a
sufficient improvement.

Could the original request be aiming to create a new worktree with
the HEAD detached at the commit pointed at by the remote-tracking
branch, instead of creating a local branch forked from it?  I am
just wondering if "-b %s" is too specific to one possible
interpretation that may contradict to what the user actually wanted
to do.

Thanks.
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.