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

From: Junio C Hamano <gitster@pobox.com>
Date: Nov 27, 2017, 06:36 UTC
Message-ID: <xmqqd144cjnw.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <20171126194356.16187-6-t.gummerer@gmail.com>

Thomas Gummerer <t.gummerer@gmail.com> writes:

Show 9 quoted lines

> 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.

Show 7 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.

Show 12 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)".