Re: [PATCH v5 5/6] worktree: add --guess-remote flag to add subcommand

[Thomas Gummerer <t.gummerer@gmail.com>]

On 11/27, Junio C Hamano wrote:
> Thomas Gummerer <t.gummerer@gmail.com> writes:
> 
> > Currently 'git worktree add <path>' creates a new branch named after the
> > basename of the <path>, that matches the HEAD of whichever worktree we
> > were on when calling "git worktree add <path>".
> >
> > It's sometimes useful to have 'git worktree add <path> behave more like
> > the dwim machinery in 'git checkout <new-branch>', i.e. check if the new
> > branch name uniquely matches the branch name of a remote-tracking
> > branch, and if so check out that branch and set the upstream to the
> > remote-tracking branch.
> 
> This paragraph was a bit hard to sympathize because it was not
> obvious that the new feature still assumes how <path> is used to
> compute the name of the new branch.  Perhaps if it were written like
> so:
> 
> 	check if the new branch name, derived from the basename of
> 	the <path>, uniquely matches the branch name of ...
> 
> I would not have had to read it twice to understand what was going
> on.

Sorry about that, will re-phrase.

> > +--[no-]guess-remote::
> > +	With `add`, instead of creating a new branch from HEAD when
> > +	`<commit-ish>` is not given, if there exists a tracking branch
> > +	in exactly one remote matching the basename of the path, base
> > +	the new branch on the remote-tracking branch, and mark the
> > +	remote-tracking branch as "upstream" from the new branch.
> > +
> 
> Would
> 
> 	git worktree add --guess-remote <path> <branch>
> 
> be an error?  It is allowed as long as <branch> and the basename of
> the <path> matches?  The option is silently ignored?  Something
> else?
> 
> I am reacting to "with `add`" part of this desciption.  I wouldn't
> be asking if it said "With `worktree add <path>` without <branch>",
> as that would make the scenario I am wondering about automatically
> "undefined".  Yes, we should strive for leaving things undefined as
> little as practically possible, but at least saying something like
> "without <branch>" explicitly there would make sure that readers
> know in what scenario this option is meant to be used a bit better.

As you mentioned below it's silently ignored.  The main reason for not
erroring out is that it would get a little bit (although not too much)
more annoying once the config variable is introduced.  If it's
strongly preferred to error out when <branch> is given I can change it
to that.

Either way I'll update the documentation.

Thanks!

> > @@ -389,6 +392,13 @@ static int add(int ac, const char **av, const char *prefix)
> >  		int n;
> >  		const char *s = worktree_basename(path, &n);
> >  		opts.new_branch = xstrndup(s, n);
> > +		if (guess_remote) {
> > +			struct object_id oid;
> > +			const char *remote =
> > +				unique_tracking_name(opts.new_branch, &oid);
> > +			if (remote)
> > +				branch = remote;
> > +		}
> >  	}
> 
> I think the answer is "silently ignored", as the above hunk is
> inside "if (ac < 2 && !opts.new_branch && !opts.detach)".
>