Re: [PATCH v5 5/6] worktree: add --guess-remote flag to add subcommand
- From
Thomas Gummerer <t.gummerer@gmail.com>
- Date
- Nov 27, 2017, 20:56 UTC
- Message-ID
- <20171127205653.GA32057@hank>
- In-Reply-To
- <xmqqd144cjnw.fsf@gitster.mtv.corp.google.com>
On 11/27, Junio C Hamano wrote:
Show 22 quoted lines
> 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.
Show 23 quoted lines
> > +--[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!
Show 16 quoted lines
> > @@ -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)".
>