patch, 4 partsworktree: Support `--orphan` when creating new worktrees

> Signed-off-by: Jacob Abel <jacobabel@nullpo.dev>
> ---
> diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt
> @@ -10,7 +10,7 @@ SYNOPSIS
>  'git worktree add' [-f] [--detach] [--checkout] [--lock [--reason <string>]]
> -                  [-b <new-branch>] <path> [<commit-ish>]
> +                  [[-b | -B] <new-branch>] <path> [<commit-ish>]
> diff --git a/builtin/worktree.c b/builtin/worktree.c
> @@ -15,9 +15,9 @@
> -#define BUILTIN_WORKTREE_ADD_USAGE \
> +#define BUILTIN_WORKTREE_ADD_USAGE                                                        \

>
> Thanks. Some historical context...
>
> Omission of -B from the synopsis was intentional[1] for the sake of brevity.
>
> There was a previous "complaint"[2] about the omission of -B in the
> synopsis. A response[3] to that message suggested that a patch
> addressing the shortcoming would probably be welcome, though it
> appears that the author of [2] never submitted such a patch.
> Summarizing [3]:
>
>     Whether or not the omission was actually a good decision is
>     questionable. [...] Of course, that decision does not assist
>     newcomers, so adding "-B" to the synopsis would help the page
>     better stand on its own.
>
> Which suggests that this patch is probably a good idea.
>
> The patch itself looks fine (though the noise-change in the position
> of the "\" line-splice seems unwarranted).
>
> [1]: https://lore.kernel.org/git/1435969052-540-17-git-send-email-sunshine@sunshineco.com/
> [2]: https://lore.kernel.org/git/alpine.LFD.2.21.1711140324580.12112@localhost.localdomain/
> [3]: https://lore.kernel.org/git/CAPig+cRc7Yqeys=oPEgPnyR4qT7qKYLbH1ifnp+6F6N+mSzNVA@mail.gmail.com/
>
> > Signed-off-by: Jacob Abel <jacobabel@nullpo.dev>
> > ---
> > diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt
> > @@ -10,7 +10,7 @@ SYNOPSIS
> >  'git worktree add' [-f] [--detach] [--checkout] [--lock [--reason <string>]]
> > -                  [-b <new-branch>] <path> [<commit-ish>]
> > +                  [[-b | -B] <new-branch>] <path> [<commit-ish>]
> > diff --git a/builtin/worktree.c b/builtin/worktree.c
> > @@ -15,9 +15,9 @@
> > -#define BUILTIN_WORKTREE_ADD_USAGE \
> > +#define BUILTIN_WORKTREE_ADD_USAGE                                                        \
>
> This is just a noise-change which (IMHO) makes it harder to spot the
> line-splice. (Presumably this whitespace change was made by an
> automated formatting tool or by a "too smart" editor?)
>
> >         N_("git worktree add [-f] [--detach] [--checkout] [--lock [--reason <string>]]\n" \
> > -          "                 [-b <new-branch>] <path> [<commit-ish>]")
> > +          "                 [[-b | -B] <new-branch>] <path> [<commit-ish>]")

> Updates the function to call `git checkout` instead of
> `git reset --hard` to simplify adding orphan support.
>
> Signed-off-by: Jacob Abel <jacobabel@nullpo.dev>
> ---
>  builtin/worktree.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/builtin/worktree.c b/builtin/worktree.c
> index b3373fbbd6..d40f771848 100644
> --- a/builtin/worktree.c
> +++ b/builtin/worktree.c
> @@ -357,7 +357,7 @@ static int checkout_worktree(const struct add_opts *opts,
>  {
>  	struct child_process cp = CHILD_PROCESS_INIT;
>  	cp.git_cmd = 1;
> -	strvec_pushl(&cp.args, "reset", "--hard", "--no-recurse-submodules", NULL);
> +	strvec_pushl(&cp.args, "checkout", "--no-recurse-submodules", NULL);
>  	if (opts->quiet)
>  		strvec_push(&cp.args, "--quiet");
>  	strvec_pushv(&cp.env, child_env->v);

> > @@ -357,7 +357,7 @@ static int checkout_worktree(const struct add_opts *opts,
> >  {
> >  	struct child_process cp = CHILD_PROCESS_INIT;
> >  	cp.git_cmd = 1;
> > -	strvec_pushl(&cp.args, "reset", "--hard", "--no-recurse-submodules", NULL);
> > +	strvec_pushl(&cp.args, "checkout", "--no-recurse-submodules", NULL);
> >  	if (opts->quiet)
> >  		strvec_push(&cp.args, "--quiet");
> >  	strvec_pushv(&cp.env, child_env->v);
>
> Won't we now start to run the post-checkout when doing this, and is that
> intended?

> +	if (!commit && !opts->implicit)
>  		die(_("invalid reference: %s"), refname);
>
>  	name = worktree_basename(path, &len);
> @@ -482,10 +487,10 @@ static int add_worktree(const char *path, const char *refname,
>  	strvec_pushf(&child_env, "%s=%s", GIT_WORK_TREE_ENVIRONMENT, path);
>  	cp.git_cmd = 1;
>
> -	if (!is_branch)
> +	if (!is_branch && commit) {
>  		strvec_pushl(&cp.args, "update-ref", "HEAD",
>  			     oid_to_hex(&commit->object.oid), NULL);

> -	else {
> +	} else {
>  		strvec_pushl(&cp.args, "symbolic-ref", "HEAD",
>  			     symref.buf, NULL);
>  		if (opts->quiet)
> @@ -516,7 +521,7 @@ static int add_worktree(const char *path, const char *refname,
>  	 * Hook failure does not warrant worktree deletion, so run hook after
>  	 * is_junk is cleared, but do return appropriate code when hook fails.
>  	 */
> -	if (!ret && opts->checkout) {
> +	if (!ret && opts->checkout && !opts->orphan_branch) {
>  		struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT;
>
>  		strvec_pushl(&opt.env, "GIT_DIR", "GIT_WORK_TREE", NULL);
> @@ -608,33 +613,52 @@ static int add(int ac, const char **av, const char *prefix)
>  	const char *opt_track = NULL;
>  	const char *lock_reason = NULL;
>  	int keep_locked = 0;
> +

>  	struct option options[] = {
> -		OPT__FORCE(&opts.force,
> -			   N_("checkout <branch> even if already checked out in other worktree"),
> -			   PARSE_OPT_NOCOMPLETE),
> +		OPT__FORCE(
> +			&opts.force,
> +			N_("checkout <branch> even if already checked out in other worktree"),
> +			PARSE_OPT_NOCOMPLETE),

>  		OPT_STRING('b', NULL, &new_branch, N_("branch"),
>  			   N_("create a new branch")),
>  		OPT_STRING('B', NULL, &new_branch_force, N_("branch"),
>  			   N_("create or reset a branch")),
> -		OPT_BOOL('d', "detach", &opts.detach, N_("detach HEAD at named commit")),
> -		OPT_BOOL(0, "checkout", &opts.checkout, N_("populate the new working tree")),
> -		OPT_BOOL(0, "lock", &keep_locked, N_("keep the new working tree locked")),
> +		OPT_STRING(0, "orphan", &opts.orphan_branch, N_("branch"),
> +			   N_("create a new unparented branch")),
> +		OPT_BOOL('d', "detach", &opts.detach,
> +			 N_("detach HEAD at named commit")),
> +		OPT_BOOL(0, "checkout", &opts.checkout,
> +			 N_("populate the new working tree")),
> +		OPT_BOOL(0, "lock", &keep_locked,
> +			 N_("keep the new working tree locked")),

>  		OPT_STRING(0, "reason", &lock_reason, N_("string"),
>  			   N_("reason for locking")),
>  		OPT__QUIET(&opts.quiet, N_("suppress progress reporting")),
>  		OPT_PASSTHRU(0, "track", &opt_track, NULL,
>  			     N_("set up tracking mode (see git-branch(1))"),
>  			     PARSE_OPT_NOARG | PARSE_OPT_OPTARG),
> -		OPT_BOOL(0, "guess-remote", &guess_remote,
> -			 N_("try to match the new branch name with a remote-tracking branch")),
> +		OPT_BOOL(
> +			0, "guess-remote", &guess_remote,
> +			N_("try to match the new branch name with a remote-tracking branch")),

>  		OPT_END()
>  	};
>
>  	memset(&opts, 0, sizeof(opts));
>  	opts.checkout = 1;
>  	ac = parse_options(ac, av, prefix, options, git_worktree_add_usage, 0);
> -	if (!!opts.detach + !!new_branch + !!new_branch_force > 1)
> -		die(_("options '%s', '%s', and '%s' cannot be used together"), "-b", "-B", "--detach");
> +
> +	opts.implicit = ac < 2;
> +
> +	if (!!opts.detach + !!new_branch + !!new_branch_force +
> +		    !!opts.orphan_branch >
> +	    1)

>
> > ...
>
> Here.
>
> > ...
>
> And here we have a stray style change, in this case conforming to our
> CodingGuidelines (it's agnostic on the former), but IMO better to keep
> this out, or split it into a "various style stuff" commit, makes this
> harder to review...

> > ...
>
> This is just a stray refactoring of existing code to not-our-usual-style
> (first arg is on the same line as the "(", rest aligned with "(").
>
> > ...
>
> Ditto, these look like they're too-long in the pre-image, but please
> resist re-flowing existing code while at it.
>
> > ...
>
> ditto.

> > +	if (!!opts.detach + !!new_branch + !!new_branch_force +
> > +		    !!opts.orphan_branch >
> > +	    1)
>
> The continued "if" is mis-indented, and that "1" is on a line of its
> own...

> Adds support for creating an orphan branch when adding a new worktree.
> This functionality is equivalent to git checkout's --orphan flag.
> [...]
> Signed-off-by: Jacob Abel <jacobabel@nullpo.dev>
> ---
> diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt
> @@ -95,6 +95,17 @@ exist, a new branch based on `HEAD` is automatically created as if
> +------------
> +$ git worktree add --orphan <branch> <path> [<commit-ish>]
> +------------
> ++
> +Create a worktree containing an orphan branch named `<branch>` based
> +on `<commit-ish>`. If `<commit-ish>` is not specified, the new orphan branch
> +will be created based on `HEAD`.
> ++
> +Note that unlike with `-b` or `-B`, this operation will succeed even if
> +`<commit-ish>` is a branch that is currently checked out somewhere else.

> @@ -222,6 +233,11 @@ This can also be set up as the default behaviour by using the
> +--orphan <new-branch>::
> +       With `add`, create a new orphan branch named `<new-branch>` in the new
> +       worktree based on `<commit-ish>`. If `<commit-ish>` is omitted, it
> +       defaults to `HEAD`.

> diff --git a/builtin/worktree.c b/builtin/worktree.c
> @@ -608,33 +613,52 @@ static int add(int ac, const char **av, const char *prefix)
>         struct option options[] = {
> +               OPT_STRING(0, "orphan", &opts.orphan_branch, N_("branch"),
> +                          N_("create a new unparented branch")),

> +       if (opts.orphan_branch && opt_track)
> +               die(_("'%s' cannot be used with '%s'"), "--orphan", "--track");
> +       if (opts.orphan_branch && !opts.checkout)
> +               die(_("'%s' cannot be used with '%s'"), "--orphan",
> +                   "--no-checkout");

> +       /*
> +        * From here on, new_branch will contain the branch to be checked out,
> +        * and new_branch_force and opts.orphan_branch will tell us which one of
> +        * -b/-B/--orphan is being used.
> +        */

> On Thu, Nov 3, 2022 at 9:07 PM Jacob Abel <jacobabel@nullpo.dev> wrote:
> > ...
>
> Are we sure we want to be modeling this after `git checkout --orphan`?
> If I understand correctly, that option has long been considered (by
> some) too clunky, which is why `git switch --orphan` was simplified to
> accept only a branch name but no commit-ish, and to start the orphan
> branch with an empty directory. My own feeling is that modeling it
> after `git switch --orphan` is probably the way to go...

> > ...
>
> The short help message for `git switch --orphan` and `git checkout
> --orphan` say simply "new unparented branch", so this message should
> probably follow suit (or consistency and to ease the job of
> translators).

> > ...
>
> Good to have these additional interlocks. I think, however, for the
> sake of translators, we should use the same terminology as the
> existing message above (i.e. "options ... cannot be used together").

>
> > +       /*
> > +        * From here on, new_branch will contain the branch to be checked out,
> > +        * and new_branch_force and opts.orphan_branch will tell us which one of
> > +        * -b/-B/--orphan is being used.
> > +        */
>
> This can probably be worded a bit differently to make it clear that
> from this point onward, those other variables are interpreted as if
> they are booleans. Moreover, we can make this even clearer by
> following the example of -B in which (by necessity due to
> parse-options) the local variable in add() is a `const char *`, but
> its counterpart in `struct add_opts` is a boolean (int).

> On 22/11/04 01:03AM, Eric Sunshine wrote:
> > On Thu, Nov 3, 2022 at 9:07 PM Jacob Abel <jacobabel@nullpo.dev> wrote:
> > Are we sure we want to be modeling this after `git checkout --orphan`?
> > If I understand correctly, that option has long been considered (by
> > some) too clunky, which is why `git switch --orphan` was simplified to
> > accept only a branch name but no commit-ish, and to start the orphan
> > branch with an empty directory. My own feeling is that modeling it
> > after `git switch --orphan` is probably the way to go...
>
> I would argue that the `git checkout --orphan` command format is preferable to
> `git switch --orphan` when creating new worktrees. Reason being that in many
> cases (except when working in a new repo), if you are trying to create a
> worktree from an orphan you will be doing it with a different commit-ish
> currently checked out in your worktree than the one you want to use for the
> orphan (or you aren't in any worktree).

> On Fri, Nov 4, 2022 at 12:42 PM Jacob Abel <jacobabel@nullpo.dev> wrote:
> > On 22/11/04 01:03AM, Eric Sunshine wrote:
> > > On Thu, Nov 3, 2022 at 9:07 PM Jacob Abel <jacobabel@nullpo.dev> wrote:
> > > Are we sure we want to be modeling this after `git checkout --orphan`?
> > > If I understand correctly, that option has long been considered (by
> > > some) too clunky, which is why `git switch --orphan` was simplified to
> > > accept only a branch name but no commit-ish, and to start the orphan
> > > branch with an empty directory. My own feeling is that modeling it
> > > after `git switch --orphan` is probably the way to go...
> >
> > I would argue that the `git checkout --orphan` command format is preferable to
> > `git switch --orphan` when creating new worktrees. Reason being that in many
> > cases (except when working in a new repo), if you are trying to create a
> > worktree from an orphan you will be doing it with a different commit-ish
> > currently checked out in your worktree than the one you want to use for the
> > orphan (or you aren't in any worktree).
>
> I guess I'm not understanding the use-case being described here or
> that this series is trying to address. In my own experience, the very,
> very few times I've used --orphan was when I needed a branch with no
> existing history (i.e. "orphan") and with no existing files. For that
> use-case, `git switch --orphan` is ideal, whereas `git checkout
> --orphan` is a bother since it requires manually removing all content
> from the directory and clearing the index.
>
> > Requiring the commit-ish to be inferred would limit the user to checking out
> > an orphan from an existing worktree (in which case they could just create a
> > new worktree normally and use `git switch --orphan` to move that to an orphan
> > branch).
>
> I'm not following what you mean by inferred commit-ish. `git switch
> --orphan` does not infer any commit-ish; it starts the orphaned branch
> with an empty directory, hence there is no commit-ish involved.
>
> The `git switch --orphan` behavior was intentionally implemented to
> "fix" what has long been considered (by some) a UX botch in the
> behavior of `git checkout --orphan`. It was argued that in the vast
> majority of cases, people wanted an orphan branch to mean both "no
> history" and "no files". So, in that sense, it feels like a step
> backward to adopt `git checkout --orphan` when introducing `git
> worktree --orphan`.
>
> But, as I said, I'm genuinely not grasping your use-case, so I'm
> having trouble understanding why you consider `git checkout --orphan`
> a better model. If you can elaborate your use-case more thoroughly,
> perhaps it would help (at least me).

> Signed-off-by: Jacob Abel <jacobabel@nullpo.dev>
> ---
>  t/t2400-worktree-add.sh | 54 +++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 54 insertions(+)
>
> diff --git a/t/t2400-worktree-add.sh b/t/t2400-worktree-add.sh
> index d587e0b20d..064e1336e2 100755
> --- a/t/t2400-worktree-add.sh
> +++ b/t/t2400-worktree-add.sh
> @@ -310,6 +310,26 @@ test_expect_success '"add" -B/--detach mutually exclusive' '
>  	test_must_fail git worktree add -B poodle --detach bamboo main
>  '
>
> +test_expect_success '"add" --orphan/-b mutually exclusive' '
> +	test_must_fail git worktree add --orphan poodle -b poodle bamboo main
> +'
> +
> +test_expect_success '"add" --orphan/-B mutually exclusive' '
> +	test_must_fail git worktree add --orphan poodle -B poodle bamboo main
> +'
> +
> +test_expect_success '"add" --orphan/--detach mutually exclusive' '
> +	test_must_fail git worktree add --orphan poodle --detach bamboo main
> +'
> +
> +test_expect_success '"add" --orphan/--no-checkout mutually exclusive' '
> +	test_must_fail git worktree add --orphan poodle --no-checkout bamboo main
> +'
> +
> +test_expect_success '"add" -B/--detach mutually exclusive' '
> +	test_must_fail git worktree add -B poodle --detach bamboo main
> +'
> +
> [...]
> +test_expect_success '"add --orphan"' '
> +	git worktree add --orphan neworphan orphandir main &&
> +	git -C orphandir symbolic-ref HEAD >actual &&
> +	echo refs/heads/neworphan >expected &&
> +	test_cmp expected actual &&

> +'
> +
> +test_expect_success '"add --orphan" fails if the branch already exists' '
> +	git worktree add -b existingbranch orphandir2 main &&
> +	test_must_fail git worktree add --orphan existingbranch orphandir3 main &&
> +	[ ! -d orphandir3 ]

> +'
> +
> +test_expect_success '"add --orphan" fails if the commit-ish doesnt exist' '
> +	test_must_fail git worktree add --orphan badcommitish orphandir4 eee2222 &&
> +	[ ! -d orphandir4 ]

> +'
> +
> +test_expect_success '"add --orphan" with empty repository' '
> +	(
> +		mkdir emptyorphanrepo &&
> +		cd emptyorphanrepo &&
> +		GIT_DIR=".git" git init --bare &&
> +		git worktree add --orphan newbranch worktreedir &&
> +		git -C worktreedir symbolic-ref HEAD >actual &&
> +		echo refs/heads/newbranch >expected &&
> +		test_cmp expected actual
> +	)

>
> On Fri, Nov 04 2022, Jacob Abel wrote:
>
> We usually add tests along with the feature, so this should be squashed
> into your 3/4.
>
> > Signed-off-by: Jacob Abel <jacobabel@nullpo.dev>
> > ---
> >  t/t2400-worktree-add.sh | 54 +++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 54 insertions(+)
> >
> > diff --git a/t/t2400-worktree-add.sh b/t/t2400-worktree-add.sh
> > index d587e0b20d..064e1336e2 100755
> > --- a/t/t2400-worktree-add.sh
> > +++ b/t/t2400-worktree-add.sh
> > @@ -310,6 +310,26 @@ test_expect_success '"add" -B/--detach mutually exclusive' '
> >  	test_must_fail git worktree add -B poodle --detach bamboo main
> >  '
> >
> > +test_expect_success '"add" --orphan/-b mutually exclusive' '
> > +	test_must_fail git worktree add --orphan poodle -b poodle bamboo main
> > +'
> > +
> > +test_expect_success '"add" --orphan/-B mutually exclusive' '
> > +	test_must_fail git worktree add --orphan poodle -B poodle bamboo main
> > +'
> > +
> > +test_expect_success '"add" --orphan/--detach mutually exclusive' '
> > +	test_must_fail git worktree add --orphan poodle --detach bamboo main
> > +'
> > +
> > +test_expect_success '"add" --orphan/--no-checkout mutually exclusive' '
> > +	test_must_fail git worktree add --orphan poodle --no-checkout bamboo main
> > +'
> > +
> > +test_expect_success '"add" -B/--detach mutually exclusive' '
> > +	test_must_fail git worktree add -B poodle --detach bamboo main
> > +'
> > +
> > [...]
> > +test_expect_success '"add --orphan"' '
> > +	git worktree add --orphan neworphan orphandir main &&
> > +	git -C orphandir symbolic-ref HEAD >actual &&
> > +	echo refs/heads/neworphan >expected &&
> > +	test_cmp expected actual &&
>
> nit: we usually do:
>
> 	echo ...
> 	git ...
> 	test_cmp
>
> So there's never doubt that the "expected" is before the command
> (i.e. not impacted by it).
>
> > +	(
> > +		cd orphandir &&
> > +		git diff main
> > +	)
>
> Here you can avoid the sub-shell, with:
>
> 	git -C orphandir diff main
>
> > +'
> > +
> > +test_expect_success '"add --orphan" fails if the branch already exists' '
> > +	git worktree add -b existingbranch orphandir2 main &&
> > +	test_must_fail git worktree add --orphan existingbranch orphandir3 main &&
> > +	[ ! -d orphandir3 ]
>
> Don't use "[", use "test", but in this case use one of the "test -d",
> "test -f" etc. wrappers in test-lib-functions.sh.
>
> > +'
> > +
> > +test_expect_success '"add --orphan" fails if the commit-ish doesnt exist' '
> > +	test_must_fail git worktree add --orphan badcommitish orphandir4 eee2222 &&
> > +	[ ! -d orphandir4 ]
>
> ditto.

> > +'
> > +
> > +test_expect_success '"add --orphan" with empty repository' '
> > +	(
> > +		mkdir emptyorphanrepo &&
> > +		cd emptyorphanrepo &&
> > +		GIT_DIR=".git" git init --bare &&
> > +		git worktree add --orphan newbranch worktreedir &&
> > +		git -C worktreedir symbolic-ref HEAD >actual &&
> > +		echo refs/heads/newbranch >expected &&
> > +		test_cmp expected actual
> > +	)
>
> Ditto we can avoid sub-shelling here, also when using sub-shells, make
> the "cd" the first command in it, if you keep it the "mkdir" should go
> outside of it.
>
> But isn't this (untested, maybe I'm missing a subtlety):
>
> 	test_when_finished "rm -rf repo" &&
> 	GIT_DIR=.git git init --bare repo &&
> 	git -C worktree ...
> 	git -C worktree/subdir ..
> 	echo refs ...
> 	test_cmp ...
>
> I.e. do we need to create the dir manually before "git init --bare"?

> While working with the worktree based git workflow, I realised that setting
> up a new git repository required switching between the traditional and
> worktree based workflows. Searching online I found a SO answer [1] which
> seemed to support this and which indicated that adding support for this should
> not be technically difficult.

> This patchset has three parts:
>   * adding `-B` to the usage docs (noticed during dev and it seemed too small
>     to justify a separate submission)
>   * switching from `git reset --hard` to `git checkout` for worktree checkout
>   * adding orphan branch functionality (as is present in `git-checkout`)
>     to `git-worktree-add`

>
> I'm pretty sure that we don't want to go the route of using
> heavyweight and heavily-featured `git checkout` as a substitute for
> lightweight `git reset --hard`. In fact, worktree functionality
> started life as a special checkout mode invoked by `git checkout
> --to`. A good deal of work[2][3][4] went into extracting that
> functionality to a standalone `git worktree add` command, and
> eventually ridding `git worktree add` of its unfortunate dependency
> upon `git checkout` as a backend[5][6][7], and ridding `git checkout`
> of its ugly intimate specialized knowledge of a newly-crafted
> worktree.
>
> The key motivation for rejecting `git checkout` as backend and
> migrating to `git reset --hard` came from Junio[8][9], and I trust
> that his observations are still pertinent.
>
> So, rather than swapping out `git reset --hard` in favor of `git
> checkout`, we probably want to stick with the already-established
> approach of adding the necessary machinery to `git worktree add`
> itself (or by refactoring `git checkout` machinery to be reusable),
> just as we did for other `git worktree` options which have `git
> checkout` counterparts, such as --track[10], --guess-remote[11],
> --[no]-checkout[12], --quiet[13], etc.

> On 22/11/04 12:33AM, Eric Sunshine wrote:
> >
> > I'm pretty sure that we don't want to go the route of using
> > heavyweight and heavily-featured `git checkout` as a substitute for
> > lightweight `git reset --hard`. In fact, worktree functionality
> > started life as a special checkout mode invoked by `git checkout
> > --to`. A good deal of work[2][3][4] went into extracting that
> > functionality to a standalone `git worktree add` command, and
> > eventually ridding `git worktree add` of its unfortunate dependency
> > upon `git checkout` as a backend[5][6][7], and ridding `git checkout`
> > of its ugly intimate specialized knowledge of a newly-crafted
> > worktree.
> >
> > The key motivation for rejecting `git checkout` as backend and
> > migrating to `git reset --hard` came from Junio[8][9], and I trust
> > that his observations are still pertinent.
> >
> > So, rather than swapping out `git reset --hard` in favor of `git
> > checkout`, we probably want to stick with the already-established
> > approach of adding the necessary machinery to `git worktree add`
> > itself (or by refactoring `git checkout` machinery to be reusable),
> > just as we did for other `git worktree` options which have `git
> > checkout` counterparts, such as --track[10], --guess-remote[11],
> > --[no]-checkout[12], --quiet[13], etc.
>
> I agree. I appreciate you sharing the history as it makes sense now why
> it is this way in particular.
>
> I think I found an alternative which was mentioned over in another reply [1]
> (running `git symbolic-ref` after running `git reset`) and I'm going to try
> implementing that for v2 and hope it doesn't introduce any complications.

> Adds support for creating an orphan branch when adding a new worktree.
> This functionality is equivalent to git switch's --orphan flag.
>
> The original reason this feature was implemented was to allow a user
> to initialise a new repository using solely the worktree oriented
> workflow. Example usage included below.
>
> $ GIT_DIR=".git" git init --bare
> $ git worktree add --orphan master master/
>
> Signed-off-by: Jacob Abel <jacobabel@nullpo.dev>
> ---
>  Documentation/git-worktree.txt | 14 +++++++-
>  builtin/worktree.c             | 64 ++++++++++++++++++++++++++++++----
>  t/t2400-worktree-add.sh        | 45 ++++++++++++++++++++++++
>  3 files changed, 115 insertions(+), 8 deletions(-)
>
> diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt
> index 4dd658012b..1310bfb564 100644
> --- a/Documentation/git-worktree.txt
> +++ b/Documentation/git-worktree.txt
> @@ -10,7 +10,7 @@ SYNOPSIS
>  --------
>  [verse]
>  'git worktree add' [-f] [--detach] [--checkout] [--lock [--reason <string>]]
> -		   [[-b | -B] <new-branch>] <path> [<commit-ish>]
> +		   [[-b | -B | --orphan] <new-branch>] <path> [<commit-ish>]
>  'git worktree list' [-v | --porcelain [-z]]
>  'git worktree lock' [--reason <string>] <worktree>
>  'git worktree move' <worktree> <new-path>
> @@ -95,6 +95,14 @@ exist, a new branch based on `HEAD` is automatically created as if
>  `-b <branch>` was given.  If `<branch>` does exist, it will be checked out
>  in the new worktree, if it's not checked out anywhere else, otherwise the
>  command will refuse to create the worktree (unless `--force` is used).
> ++
> +------------
> +$ git worktree add --orphan <branch> <path>
> +------------
> ++
> +Create a worktree containing an orphan branch named `<branch>` with a
> +clean working directory.  See `--orphan` in linkgit:git-switch[1] for
> +more details.

> +test_expect_success '"add" --orphan/-b mutually exclusive' '
> +	test_must_fail git worktree add --orphan poodle -b poodle bamboo
> +'
> +
> +test_expect_success '"add" --orphan/-B mutually exclusive' '
> +	test_must_fail git worktree add --orphan poodle -B poodle bamboo
> +'
> +
> +test_expect_success '"add" --orphan/--detach mutually exclusive' '
> +	test_must_fail git worktree add --orphan poodle --detach bamboo
> +'
> +
> +test_expect_success '"add" --orphan/--no-checkout mutually exclusive' '
> +	test_must_fail git worktree add --orphan poodle --no-checkout bamboo
> +'
> +
> +test_expect_success '"add" -B/--detach mutually exclusive' '
> +	test_must_fail git worktree add -B poodle --detach bamboo main
> +'
> +

> +test_expect_success '"add --orphan" fails if the branch already exists' '
> +	test_when_finished "git branch -D existingbranch" &&
> +	test_when_finished "git worktree remove -f -f orphandir" &&
> +	git worktree add -b existingbranch orphandir main &&
> +	test_must_fail git worktree add --orphan existingbranch orphandir2 &&
> +	test ! -d orphandir2

> On Thu, Nov 10 2022, Jacob Abel wrote:
> > Adds support for creating an orphan branch when adding a new worktree.
> > This functionality is equivalent to git switch's --orphan flag.
> >
> > The original reason this feature was implemented was to allow a user
> > to initialise a new repository using solely the worktree oriented
> > workflow. Example usage included below.
> >
> > $ GIT_DIR=".git" git init --bare
> > $ git worktree add --orphan master master/
> >
> > Signed-off-by: Jacob Abel <jacobabel@nullpo.dev>
> > ---
> > +Create a worktree containing an orphan branch named `<branch>` with a
> > +clean working directory.  See `--orphan` in linkgit:git-switch[1] for
> > +more details.
>
> Seeing as "git switch" is still marked "EXPERIMENTAL", it may be prudent
> in general to avoid linking to it in lieu of "git checkout".
>
> In this case in particular though the "more details" are almost
> completely absent from the "git-switch" docs, and they don't (which is
> their won flaw) link to the more detailed "git-checkout" docs.
>
> But for this patch, it seems much better to link to the "checkout" docs,
> no?

> > +test_expect_success '"add" --orphan/-b mutually exclusive' '
> > +     test_must_fail git worktree add --orphan poodle -b poodle bamboo
> > +'
> > +
> > +test_expect_success '"add" --orphan/-B mutually exclusive' '
> > +     test_must_fail git worktree add --orphan poodle -B poodle bamboo
> > +'
> > +
> > +test_expect_success '"add" --orphan/--detach mutually exclusive' '
> > +     test_must_fail git worktree add --orphan poodle --detach bamboo
> > +'
> > +
> > +test_expect_success '"add" --orphan/--no-checkout mutually exclusive' '
> > +     test_must_fail git worktree add --orphan poodle --no-checkout bamboo
> > +'
> > +
> > +test_expect_success '"add" -B/--detach mutually exclusive' '
> > +     test_must_fail git worktree add -B poodle --detach bamboo main
> > +'
> > +
>
> This would be much better as a for-loop:
>
> for opt in -b -B ...
> do
>         test_expect_success "...$opt" '<test here, uses $opt>'
> done
>
> Note the ""-quotes for the description, and '' for the test, that's not
> a mistake, we eval() the latter.

> On Tue, Nov 15, 2022 at 4:13 PM Ævar Arnfjörð Bjarmason
> <avarab@gmail.com> wrote:
>> On Thu, Nov 10 2022, Jacob Abel wrote:
>> > Adds support for creating an orphan branch when adding a new worktree.
>> > This functionality is equivalent to git switch's --orphan flag.
>> >
>> > The original reason this feature was implemented was to allow a user
>> > to initialise a new repository using solely the worktree oriented
>> > workflow. Example usage included below.
>> >
>> > $ GIT_DIR=".git" git init --bare
>> > $ git worktree add --orphan master master/
>> >
>> > Signed-off-by: Jacob Abel <jacobabel@nullpo.dev>
>> > ---
>> > +Create a worktree containing an orphan branch named `<branch>` with a
>> > +clean working directory.  See `--orphan` in linkgit:git-switch[1] for
>> > +more details.
>>
>> Seeing as "git switch" is still marked "EXPERIMENTAL", it may be prudent
>> in general to avoid linking to it in lieu of "git checkout".
>>
>> In this case in particular though the "more details" are almost
>> completely absent from the "git-switch" docs, and they don't (which is
>> their won flaw) link to the more detailed "git-checkout" docs.
>>
>> But for this patch, it seems much better to link to the "checkout" docs,
>> no?
>
> Sorry, no. The important point here is that the --orphan option being
> added to `git worktree add` closely follows the behavior of `git
> switch --orphan`, which is quite different from the behavior of `git
> checkout --orphan`.
>
> The `git switch --orphan` documentation doesn't seem particularly
> lacking; it correctly describes the (very) simplified behavior of that
> command over `git checkout --orphan`. I might agree that there isn't
> much reason to link to git-switch for "more details", though, since
> there isn't really anything else that needs to be said.

> If we did want to say something else here, we might copy one sentence
> from the `git checkout --orphan` documentation:
>
>     The first commit made on this new branch will have no parents and
>     it will be the root of a new history totally disconnected from all
>     the other branches and commits.
>
> The same sentence could be added to `git switch --orphan`
> documentation, but that's outside the scope of this patch series (thus
> can be done later by someone).

>> > +test_expect_success '"add" --orphan/-b mutually exclusive' '
>> > +     test_must_fail git worktree add --orphan poodle -b poodle bamboo
>> > +'
>> > +
>> > +test_expect_success '"add" --orphan/-B mutually exclusive' '
>> > +     test_must_fail git worktree add --orphan poodle -B poodle bamboo
>> > +'
>> > +
>> > +test_expect_success '"add" --orphan/--detach mutually exclusive' '
>> > +     test_must_fail git worktree add --orphan poodle --detach bamboo
>> > +'
>> > +
>> > +test_expect_success '"add" --orphan/--no-checkout mutually exclusive' '
>> > +     test_must_fail git worktree add --orphan poodle --no-checkout bamboo
>> > +'
>> > +
>> > +test_expect_success '"add" -B/--detach mutually exclusive' '
>> > +     test_must_fail git worktree add -B poodle --detach bamboo main
>> > +'
>> > +
>>
>> This would be much better as a for-loop:
>>
>> for opt in -b -B ...
>> do
>>         test_expect_success "...$opt" '<test here, uses $opt>'
>> done
>>
>> Note the ""-quotes for the description, and '' for the test, that's not
>> a mistake, we eval() the latter.
>
> Such a loop would need to be more complex than this, wouldn't it, to
> account for all the combinations? I'd normally agree about the loop,
> but given that it requires extra complexity, I don't really mind
> seeing the individual tests spelled out manually in this case; they're
> dead simple to understand as written. I don't feel strongly either
> way, but I also don't want to ask for extra work from the patch author
> for a subjective change.

> On Tue, Nov 15 2022, Eric Sunshine wrote:
> > On Tue, Nov 15, 2022 at 4:13 PM Ævar Arnfjörð Bjarmason
> > <avarab@gmail.com> wrote:
> >> But for this patch, it seems much better to link to the "checkout" docs,
> >> no?
> >
> > Sorry, no. The important point here is that the --orphan option being
> > added to `git worktree add` closely follows the behavior of `git
> > switch --orphan`, which is quite different from the behavior of `git
> > checkout --orphan`.
> >
> > The `git switch --orphan` documentation doesn't seem particularly
> > lacking; it correctly describes the (very) simplified behavior of that
> > command over `git checkout --orphan`. I might agree that there isn't
> > much reason to link to git-switch for "more details", though, since
> > there isn't really anything else that needs to be said.
>
> Aside from what it says now: 1/2 of what I'm saying is that linking to
> it while it says it's "EXPERIMENTAL" might be either jumping the gun.
>
> Or maybe we should just declare it non-"EXPERIMENTAL", but in any case
> this unrelated topic might want to avoid that altogether and just link
> to the "checkout" version.

> > If we did want to say something else here, we might copy one sentence
> > from the `git checkout --orphan` documentation:
> >
> >     The first commit made on this new branch will have no parents and
> >     it will be the root of a new history totally disconnected from all
> >     the other branches and commits.
> >
> > The same sentence could be added to `git switch --orphan`
> > documentation, but that's outside the scope of this patch series (thus
> > can be done later by someone).
>
> I think I was partially confused by skimming the SYNOPSIS and thinking
> this supported <start-point> like checkout, which as I found in
> https://lore.kernel.org/git/221115.86edu3kfqz.gmgdl@evledraar.gmail.com/
> just seems to be a missing assertion where we want to die() if that's
> provided in this mode.

> What I also found a bit confusing (but maybe it's just me) is that the
> "with a clean working directory" seemed at first to be drawing a
> distinction between this behavior and that of "git switch", but from
> poking at it some more it seems to be expressing "this is like git
> switch's --orphan" with that.

> I think instead of "clean working tree" it would be better to talk about
> "tracked files", as "git switch --orphan" does, which AFAICT is what it
> means. But then again the reason "switch" does that is because you have
> *existing* tracked files, which inherently doesn't apply for "worktree".
>
> Hrm.
>
> So, I guess it depends on your mental model of this operation, but at
> least I think it's more intuitive to explain it in terms of "git
> checkout --orphan", not "git switch --orphan". I.e.:
>
>         Create a worktree containing an orphan branch named
>         `<branch>`. This works like linkgit:git-checkout[1]'s `--orphan'
>         option, except '<start-point>` isn't supported, and the "clear
>         the index" doesn't apply (as "worktree add" will always have a
>         new index)".
>
> Whereas defining this in terms of git-switch's "All tracked files are
> removed" might just be more confusing. What files? Since it's "worktree
> add" there weren't any in the first place.

> >> This would be much better as a for-loop:
> >
> > Such a loop would need to be more complex than this, wouldn't it, to
> > account for all the combinations? I'd normally agree about the loop,
> > but given that it requires extra complexity, I don't really mind
> > seeing the individual tests spelled out manually in this case; they're
> > dead simple to understand as written. I don't feel strongly either
> > way, but I also don't want to ask for extra work from the patch author
> > for a subjective change.
>
> Yeah, it's probably not worth it. This is partially cleaning up existing
> tests, but maybe:
>
>         -test_expect_success '"add" -b/-B mutually exclusive' '
>         -       test_must_fail git worktree add -b poodle -B poodle bamboo main
>         -'
>         -
>         +test_wt_add_excl() {
>         +       local opts="$@" &&
>         +       test_expect_success "'worktree add' with '$opts' has mutually exclusive options" '
>         +               test_must_fail git worktree add $opts
>         +       '
>         +}
>         +test_wt_add_excl -b poodle -B poodle bamboo main
>         +test_wt_add_excl -b poodle --orphan poodle bamboo

> I re-arranged that a bit, but probably not worth a loop. I *did* spot in
> doing that that if I sort the options I end up with a duplicate test,
> i.e. we test "-B poodle --detach bamboo main" twice.
>
> That seems to be added by mistake in 2/2, i.e. it's the existing test
> you can see in the diff context, just added at the end.

> On Tue, Nov 15, 2022 at 6:27 PM Ævar Arnfjörð Bjarmason
> <avarab@gmail.com> wrote:
> > On Tue, Nov 15 2022, Eric Sunshine wrote:
> > > On Tue, Nov 15, 2022 at 4:13 PM Ævar Arnfjörð Bjarmason
> > > <avarab@gmail.com> wrote:
> > >> But for this patch, it seems much better to link to the "checkout" docs,
> > >> no?
> > >
> > > Sorry, no. The important point here is that the --orphan option being
> > > added to `git worktree add` closely follows the behavior of `git
> > > switch --orphan`, which is quite different from the behavior of `git
> > > checkout --orphan`.
> > >
> > > The `git switch --orphan` documentation doesn't seem particularly
> > > lacking; it correctly describes the (very) simplified behavior of that
> > > command over `git checkout --orphan`. I might agree that there isn't
> > > much reason to link to git-switch for "more details", though, since
> > > there isn't really anything else that needs to be said.
> >
> > Aside from what it says now: 1/2 of what I'm saying is that linking to
> > it while it says it's "EXPERIMENTAL" might be either jumping the gun.
> >
> > Or maybe we should just declare it non-"EXPERIMENTAL", but in any case
> > this unrelated topic might want to avoid that altogether and just link
> > to the "checkout" version.
>
> Even better would be for the documentation added by this patch to be
> self-contained and not bother linking anywhere to further explain
> --orphan. That would satisfy your concern, I think, as well as my
> concern that `git checkout --orphan` documentation is inappropriate
> for `git worktree add --orphan`.
>
> > > If we did want to say something else here, we might copy one sentence
> > > from the `git checkout --orphan` documentation:
> > >
> > >     The first commit made on this new branch will have no parents and
> > >     it will be the root of a new history totally disconnected from all
> > >     the other branches and commits.
> > >
> > > The same sentence could be added to `git switch --orphan`
> > > documentation, but that's outside the scope of this patch series (thus
> > > can be done later by someone).
> >
> > I think I was partially confused by skimming the SYNOPSIS and thinking
> > this supported <start-point> like checkout, which as I found in
> > https://lore.kernel.org/git/221115.86edu3kfqz.gmgdl@evledraar.gmail.com/
> > just seems to be a missing assertion where we want to die() if that's
> > provided in this mode.
>
> I haven't read v3 yet, so I wasn't aware that the SYNOPSIS hadn't been
> updated to match the reworked --orphan behavior implemented by v3, but
> I can certainly understand how that would have led you astray. You're
> quite correct that the SYNOPSIS should not be saying that <commit-ish>
> is allowed with --orphan.
>
> > What I also found a bit confusing (but maybe it's just me) is that the
> > "with a clean working directory" seemed at first to be drawing a
> > distinction between this behavior and that of "git switch", but from
> > poking at it some more it seems to be expressing "this is like git
> > switch's --orphan" with that.
>
> "clean working directory" may indeed be ambiguous and confusing. It's
> not necessarily clear if it means "no changes to tracked files" or "no
> files in directory". We should use more precise terminology.
>
> > I think instead of "clean working tree" it would be better to talk about
> > "tracked files", as "git switch --orphan" does, which AFAICT is what it
> > means. But then again the reason "switch" does that is because you have
> > *existing* tracked files, which inherently doesn't apply for "worktree".
> >
> > Hrm.
> >
> > So, I guess it depends on your mental model of this operation, but at
> > least I think it's more intuitive to explain it in terms of "git
> > checkout --orphan", not "git switch --orphan". I.e.:
> >
> >         Create a worktree containing an orphan branch named
> >         `<branch>`. This works like linkgit:git-checkout[1]'s `--orphan'
> >         option, except '<start-point>` isn't supported, and the "clear
> >         the index" doesn't apply (as "worktree add" will always have a
> >         new index)".
> >
> > Whereas defining this in terms of git-switch's "All tracked files are
> > removed" might just be more confusing. What files? Since it's "worktree
> > add" there weren't any in the first place.
>
> I would find it clearer not to talk about or reference `git checkout
> --orphan` at all. And, as mentioned above, it shouldn't need to
> reference `git switch --orphan` either. How about something like this
> for the description of the `add` subcommand?
>
>     Create a worktree containing no files and with an empty index, and
>     associated with a new orphan branch named `<branch>`. The first
>     commit made on this new branch will have no parents and will be
>     the root of a new history disconnected from any other branches.
>
> And then to document the --orphan command:
>
>     With `add`, make the new worktree and index empty, and associate
>     the worktree with a new orphan branch named `<new-branch>`.

>
> On Tue, Nov 15 2022, Eric Sunshine wrote:
>
> > On Tue, Nov 15, 2022 at 4:13 PM Ævar Arnfjörð Bjarmason
> > <avarab@gmail.com> wrote:
> >> On Thu, Nov 10 2022, Jacob Abel wrote:
> >> > Adds support for creating an orphan branch when adding a new worktree.
> >> > This functionality is equivalent to git switch's --orphan flag.
> >> >
> >> > The original reason this feature was implemented was to allow a user
> >> > to initialise a new repository using solely the worktree oriented
> >> > workflow. Example usage included below.
> >> >
> >> > $ GIT_DIR=".git" git init --bare
> >> > $ git worktree add --orphan master master/
> >> >
> >> > Signed-off-by: Jacob Abel <jacobabel@nullpo.dev>
> >> > ---
> >> > +Create a worktree containing an orphan branch named `<branch>` with a
> >> > +clean working directory.  See `--orphan` in linkgit:git-switch[1] for
> >> > +more details.
> >>
> >> Seeing as "git switch" is still marked "EXPERIMENTAL", it may be prudent
> >> in general to avoid linking to it in lieu of "git checkout".
> >>
> >> In this case in particular though the "more details" are almost
> >> completely absent from the "git-switch" docs, and they don't (which is
> >> their won flaw) link to the more detailed "git-checkout" docs.
> >>
> >> But for this patch, it seems much better to link to the "checkout" docs,
> >> no?
> >
> > Sorry, no. The important point here is that the --orphan option being
> > added to `git worktree add` closely follows the behavior of `git
> > switch --orphan`, which is quite different from the behavior of `git
> > checkout --orphan`.
> >
> > The `git switch --orphan` documentation doesn't seem particularly
> > lacking; it correctly describes the (very) simplified behavior of that
> > command over `git checkout --orphan`. I might agree that there isn't
> > much reason to link to git-switch for "more details", though, since
> > there isn't really anything else that needs to be said.
>
> Aside from what it says now: 1/2 of what I'm saying is that linking to
> it while it says it's "EXPERIMENTAL" might be either jumping the gun.
>
> Or maybe we should just declare it non-"EXPERIMENTAL", but in any case
> this unrelated topic might want to avoid that altogether and just link
> to the "checkout" version.
>
> A quick grep of our docs (for linkgit:git-switch) that this would be the
> first mention outside of user-manual.txt where we link to it when it's
> not in the context of "checkout or switch", or where we're explaining
> something switch-specific (i.e. the "suggestDetachingHead" advice).
>
> Having said that I don't really care, just a suggestion...
>
> > If we did want to say something else here, we might copy one sentence
> > from the `git checkout --orphan` documentation:
> >
> >     The first commit made on this new branch will have no parents and
> >     it will be the root of a new history totally disconnected from all
> >     the other branches and commits.
> >
> > The same sentence could be added to `git switch --orphan`
> > documentation, but that's outside the scope of this patch series (thus
> > can be done later by someone).
>
> I think I was partially confused by skimming the SYNOPSIS and thinking
> this supported <start-point> like checkout, which as I found in
> https://lore.kernel.org/git/221115.86edu3kfqz.gmgdl@evledraar.gmail.com/
> just seems to be a missing assertion where we want to die() if that's
> provided in this mode.
>
> What I also found a bit confusing (but maybe it's just me) is that the
> "with a clean working directory" seemed at first to be drawing a
> distinction between this behavior and that of "git switch", but from
> poking at it some more it seems to be expressing "this is like git
> switch's --orphan" with that.
>
> I think instead of "clean working tree" it would be better to talk about
> "tracked files", as "git switch --orphan" does, which AFAICT is what it
> means. But then again the reason "switch" does that is because you have
> *existing* tracked files, which inherently doesn't apply for "worktree".
>
> Hrm.
>
> So, I guess it depends on your mental model of this operation, but at
> least I think it's more intuitive to explain it in terms of "git
> checkout --orphan", not "git switch --orphan". I.e.:
>
> 	Create a worktree containing an orphan branch named
> 	`<branch>`. This works like linkgit:git-checkout[1]'s `--orphan'
> 	option, except '<start-point>` isn't supported, and the "clear
> 	the index" doesn't apply (as "worktree add" will always have a
> 	new index)".
>
> Whereas defining this in terms of git-switch's "All tracked files are
> removed" might just be more confusing. What files? Since it's "worktree
> add" there weren't any in the first place.
>
> Anyway, I don't mind it as it is, but maybe the above write-up helps for
> #leftoverbits if we ever want to unify these docs. I.e. AFAICT we could:
>
>  * Link from git-worktree to git-checkout, saying the above
>  * Link from git-switch to git-checkout, ditto, but that we also "remove
>    tracked files [of the current HEAD]".

>
> >> > +test_expect_success '"add" --orphan/-b mutually exclusive' '
> >> > +     test_must_fail git worktree add --orphan poodle -b poodle bamboo
> >> > +'
> >> > +
> >> > +test_expect_success '"add" --orphan/-B mutually exclusive' '
> >> > +     test_must_fail git worktree add --orphan poodle -B poodle bamboo
> >> > +'
> >> > +
> >> > +test_expect_success '"add" --orphan/--detach mutually exclusive' '
> >> > +     test_must_fail git worktree add --orphan poodle --detach bamboo
> >> > +'
> >> > +
> >> > +test_expect_success '"add" --orphan/--no-checkout mutually exclusive' '
> >> > +     test_must_fail git worktree add --orphan poodle --no-checkout bamboo
> >> > +'
> >> > +
> >> > +test_expect_success '"add" -B/--detach mutually exclusive' '
> >> > +     test_must_fail git worktree add -B poodle --detach bamboo main
> >> > +'
> >> > +
> >>
> >> This would be much better as a for-loop:
> >>
> >> for opt in -b -B ...
> >> do
> >>         test_expect_success "...$opt" '<test here, uses $opt>'
> >> done
> >>
> >> Note the ""-quotes for the description, and '' for the test, that's not
> >> a mistake, we eval() the latter.
> >
> > Such a loop would need to be more complex than this, wouldn't it, to
> > account for all the combinations? I'd normally agree about the loop,
> > but given that it requires extra complexity, I don't really mind
> > seeing the individual tests spelled out manually in this case; they're
> > dead simple to understand as written. I don't feel strongly either
> > way, but I also don't want to ask for extra work from the patch author
> > for a subjective change.
>
> Yeah, it's probably not worth it. This is partially cleaning up existing
> tests, but maybe:
>
> 	diff --git a/t/t2400-worktree-add.sh b/t/t2400-worktree-add.sh
> 	index 93c340f4aff..5acfd48f418 100755
> 	--- a/t/t2400-worktree-add.sh
> 	+++ b/t/t2400-worktree-add.sh
> 	@@ -298,37 +298,21 @@ test_expect_success '"add" no auto-vivify with --detach and <branch> omitted' '
> 	 	test_must_fail git -C mish/mash symbolic-ref HEAD
> 	 '
>
> 	-test_expect_success '"add" -b/-B mutually exclusive' '
> 	-	test_must_fail git worktree add -b poodle -B poodle bamboo main
> 	-'
> 	-
> 	-test_expect_success '"add" -b/--detach mutually exclusive' '
> 	-	test_must_fail git worktree add -b poodle --detach bamboo main
> 	-'
> 	-
> 	-test_expect_success '"add" -B/--detach mutually exclusive' '
> 	-	test_must_fail git worktree add -B poodle --detach bamboo main
> 	-'
> 	-
> 	-test_expect_success '"add" --orphan/-b mutually exclusive' '
> 	-	test_must_fail git worktree add --orphan poodle -b poodle bamboo
> 	-'
> 	-
> 	-test_expect_success '"add" --orphan/-B mutually exclusive' '
> 	-	test_must_fail git worktree add --orphan poodle -B poodle bamboo
> 	-'
> 	-
> 	-test_expect_success '"add" --orphan/--detach mutually exclusive' '
> 	-	test_must_fail git worktree add --orphan poodle --detach bamboo
> 	-'
> 	-
> 	-test_expect_success '"add" --orphan/--no-checkout mutually exclusive' '
> 	-	test_must_fail git worktree add --orphan poodle --no-checkout bamboo
> 	-'
> 	-
> 	-test_expect_success '"add" -B/--detach mutually exclusive' '
> 	-	test_must_fail git worktree add -B poodle --detach bamboo main
> 	-'
> 	+test_wt_add_excl() {
> 	+	local opts="$@" &&
> 	+	test_expect_success "'worktree add' with '$opts' has mutually exclusive options" '
> 	+		test_must_fail git worktree add $opts
> 	+	'
> 	+}
> 	+test_wt_add_excl -b poodle -B poodle bamboo main
> 	+test_wt_add_excl -b poodle --orphan poodle bamboo
> 	+test_wt_add_excl -b poodle --detach bamboo main
> 	+test_wt_add_excl -B poodle --detach bamboo main
> 	+test_wt_add_excl -B poodle --detach bamboo main
> 	+test_wt_add_excl -B poodle --orphan poodle bamboo
> 	+test_wt_add_excl --orphan poodle --detach bamboo
> 	+test_wt_add_excl --orphan poodle --no-checkout bamboo
> 	+test_wt_add_excl --orphan poodle bamboo main
>
> 	 test_expect_success '"add -B" fails if the branch is checked out' '
> 	 	git rev-parse newmain >before &&
>
> I re-arranged that a bit, but probably not worth a loop. I *did* spot in
> doing that that if I sort the options I end up with a duplicate test,
> i.e. we test "-B poodle --detach bamboo main" twice.
>
> That seems to be added by mistake in 2/2, i.e. it's the existing test
> you can see in the diff context, just added at the end.

> On 22/11/15 11:35PM, Ævar Arnfjörð Bjarmason wrote:
>>
>> On Tue, Nov 15 2022, Eric Sunshine wrote:
>>
>> > On Tue, Nov 15, 2022 at 4:13 PM Ævar Arnfjörð Bjarmason
>> > <avarab@gmail.com> wrote:
>> >> On Thu, Nov 10 2022, Jacob Abel wrote:
>> >> > Adds support for creating an orphan branch when adding a new worktree.
>> >> > This functionality is equivalent to git switch's --orphan flag.
>> >> >
>> >> > The original reason this feature was implemented was to allow a user
>> >> > to initialise a new repository using solely the worktree oriented
>> >> > workflow. Example usage included below.
>> >> >
>> >> > $ GIT_DIR=".git" git init --bare
>> >> > $ git worktree add --orphan master master/
>> >> >
>> >> > Signed-off-by: Jacob Abel <jacobabel@nullpo.dev>
>> >> > ---
>> >> > +Create a worktree containing an orphan branch named `<branch>` with a
>> >> > +clean working directory.  See `--orphan` in linkgit:git-switch[1] for
>> >> > +more details.
>> >>
>> >> Seeing as "git switch" is still marked "EXPERIMENTAL", it may be prudent
>> >> in general to avoid linking to it in lieu of "git checkout".
>> >>
>> >> In this case in particular though the "more details" are almost
>> >> completely absent from the "git-switch" docs, and they don't (which is
>> >> their won flaw) link to the more detailed "git-checkout" docs.
>> >>
>> >> But for this patch, it seems much better to link to the "checkout" docs,
>> >> no?
>> >
>> > Sorry, no. The important point here is that the --orphan option being
>> > added to `git worktree add` closely follows the behavior of `git
>> > switch --orphan`, which is quite different from the behavior of `git
>> > checkout --orphan`.
>> >
>> > The `git switch --orphan` documentation doesn't seem particularly
>> > lacking; it correctly describes the (very) simplified behavior of that
>> > command over `git checkout --orphan`. I might agree that there isn't
>> > much reason to link to git-switch for "more details", though, since
>> > there isn't really anything else that needs to be said.
>>
>> Aside from what it says now: 1/2 of what I'm saying is that linking to
>> it while it says it's "EXPERIMENTAL" might be either jumping the gun.
>>
>> Or maybe we should just declare it non-"EXPERIMENTAL", but in any case
>> this unrelated topic might want to avoid that altogether and just link
>> to the "checkout" version.
>>
>> A quick grep of our docs (for linkgit:git-switch) that this would be the
>> first mention outside of user-manual.txt where we link to it when it's
>> not in the context of "checkout or switch", or where we're explaining
>> something switch-specific (i.e. the "suggestDetachingHead" advice).
>>
>> Having said that I don't really care, just a suggestion...
>>
>> > If we did want to say something else here, we might copy one sentence
>> > from the `git checkout --orphan` documentation:
>> >
>> >     The first commit made on this new branch will have no parents and
>> >     it will be the root of a new history totally disconnected from all
>> >     the other branches and commits.
>> >
>> > The same sentence could be added to `git switch --orphan`
>> > documentation, but that's outside the scope of this patch series (thus
>> > can be done later by someone).
>>
>> I think I was partially confused by skimming the SYNOPSIS and thinking
>> this supported <start-point> like checkout, which as I found in
>> https://lore.kernel.org/git/221115.86edu3kfqz.gmgdl@evledraar.gmail.com/
>> just seems to be a missing assertion where we want to die() if that's
>> provided in this mode.
>>
>> What I also found a bit confusing (but maybe it's just me) is that the
>> "with a clean working directory" seemed at first to be drawing a
>> distinction between this behavior and that of "git switch", but from
>> poking at it some more it seems to be expressing "this is like git
>> switch's --orphan" with that.
>>
>> I think instead of "clean working tree" it would be better to talk about
>> "tracked files", as "git switch --orphan" does, which AFAICT is what it
>> means. But then again the reason "switch" does that is because you have
>> *existing* tracked files, which inherently doesn't apply for "worktree".
>>
>> Hrm.
>>
>> So, I guess it depends on your mental model of this operation, but at
>> least I think it's more intuitive to explain it in terms of "git
>> checkout --orphan", not "git switch --orphan". I.e.:
>>
>> 	Create a worktree containing an orphan branch named
>> 	`<branch>`. This works like linkgit:git-checkout[1]'s `--orphan'
>> 	option, except '<start-point>` isn't supported, and the "clear
>> 	the index" doesn't apply (as "worktree add" will always have a
>> 	new index)".
>>
>> Whereas defining this in terms of git-switch's "All tracked files are
>> removed" might just be more confusing. What files? Since it's "worktree
>> add" there weren't any in the first place.
>>
>> Anyway, I don't mind it as it is, but maybe the above write-up helps for
>> #leftoverbits if we ever want to unify these docs. I.e. AFAICT we could:
>>
>>  * Link from git-worktree to git-checkout, saying the above
>>  * Link from git-switch to git-checkout, ditto, but that we also "remove
>>    tracked files [of the current HEAD]".
>
> Apologies for the mistake in the SYNOPSIS. As mentioned in the other replies
> I've updated it as you indicated to correct that.
>
> As for a path forwards on the referencing of either git-checkout or git-switch
> from git-worktree, I think I'm leaning towards Eric's approach (in his reply
> to this message) where we don't reference either and fully outline the
> behavior itself.

>>
>> >> > +test_expect_success '"add" --orphan/-b mutually exclusive' '
>> >> > +     test_must_fail git worktree add --orphan poodle -b poodle bamboo
>> >> > +'
>> >> > +
>> >> > +test_expect_success '"add" --orphan/-B mutually exclusive' '
>> >> > +     test_must_fail git worktree add --orphan poodle -B poodle bamboo
>> >> > +'
>> >> > +
>> >> > +test_expect_success '"add" --orphan/--detach mutually exclusive' '
>> >> > +     test_must_fail git worktree add --orphan poodle --detach bamboo
>> >> > +'
>> >> > +
>> >> > +test_expect_success '"add" --orphan/--no-checkout mutually exclusive' '
>> >> > +     test_must_fail git worktree add --orphan poodle --no-checkout bamboo
>> >> > +'
>> >> > +
>> >> > +test_expect_success '"add" -B/--detach mutually exclusive' '
>> >> > +     test_must_fail git worktree add -B poodle --detach bamboo main
>> >> > +'
>> >> > +
>> >>
>> >> This would be much better as a for-loop:
>> >>
>> >> for opt in -b -B ...
>> >> do
>> >>         test_expect_success "...$opt" '<test here, uses $opt>'
>> >> done
>> >>
>> >> Note the ""-quotes for the description, and '' for the test, that's not
>> >> a mistake, we eval() the latter.
>> >
>> > Such a loop would need to be more complex than this, wouldn't it, to
>> > account for all the combinations? I'd normally agree about the loop,
>> > but given that it requires extra complexity, I don't really mind
>> > seeing the individual tests spelled out manually in this case; they're
>> > dead simple to understand as written. I don't feel strongly either
>> > way, but I also don't want to ask for extra work from the patch author
>> > for a subjective change.
>>
>> Yeah, it's probably not worth it. This is partially cleaning up existing
>> tests, but maybe:
>>
>> 	diff --git a/t/t2400-worktree-add.sh b/t/t2400-worktree-add.sh
>> 	index 93c340f4aff..5acfd48f418 100755
>> 	--- a/t/t2400-worktree-add.sh
>> 	+++ b/t/t2400-worktree-add.sh
>> 	@@ -298,37 +298,21 @@ test_expect_success '"add" no auto-vivify with --detach and <branch> omitted' '
>> 	 	test_must_fail git -C mish/mash symbolic-ref HEAD
>> 	 '
>>
>> 	-test_expect_success '"add" -b/-B mutually exclusive' '
>> 	-	test_must_fail git worktree add -b poodle -B poodle bamboo main
>> 	-'
>> 	-
>> 	-test_expect_success '"add" -b/--detach mutually exclusive' '
>> 	-	test_must_fail git worktree add -b poodle --detach bamboo main
>> 	-'
>> 	-
>> 	-test_expect_success '"add" -B/--detach mutually exclusive' '
>> 	-	test_must_fail git worktree add -B poodle --detach bamboo main
>> 	-'
>> 	-
>> 	-test_expect_success '"add" --orphan/-b mutually exclusive' '
>> 	-	test_must_fail git worktree add --orphan poodle -b poodle bamboo
>> 	-'
>> 	-
>> 	-test_expect_success '"add" --orphan/-B mutually exclusive' '
>> 	-	test_must_fail git worktree add --orphan poodle -B poodle bamboo
>> 	-'
>> 	-
>> 	-test_expect_success '"add" --orphan/--detach mutually exclusive' '
>> 	-	test_must_fail git worktree add --orphan poodle --detach bamboo
>> 	-'
>> 	-
>> 	-test_expect_success '"add" --orphan/--no-checkout mutually exclusive' '
>> 	-	test_must_fail git worktree add --orphan poodle --no-checkout bamboo
>> 	-'
>> 	-
>> 	-test_expect_success '"add" -B/--detach mutually exclusive' '
>> 	-	test_must_fail git worktree add -B poodle --detach bamboo main
>> 	-'
>> 	+test_wt_add_excl() {
>> 	+	local opts="$@" &&
>> 	+	test_expect_success "'worktree add' with '$opts' has mutually exclusive options" '
>> 	+		test_must_fail git worktree add $opts
>> 	+	'
>> 	+}
>> 	+test_wt_add_excl -b poodle -B poodle bamboo main
>> 	+test_wt_add_excl -b poodle --orphan poodle bamboo
>> 	+test_wt_add_excl -b poodle --detach bamboo main
>> 	+test_wt_add_excl -B poodle --detach bamboo main
>> 	+test_wt_add_excl -B poodle --detach bamboo main
>> 	+test_wt_add_excl -B poodle --orphan poodle bamboo
>> 	+test_wt_add_excl --orphan poodle --detach bamboo
>> 	+test_wt_add_excl --orphan poodle --no-checkout bamboo
>> 	+test_wt_add_excl --orphan poodle bamboo main
>>
>> 	 test_expect_success '"add -B" fails if the branch is checked out' '
>> 	 	git rev-parse newmain >before &&
>>
>> I re-arranged that a bit, but probably not worth a loop. I *did* spot in
>> doing that that if I sort the options I end up with a duplicate test,
>> i.e. we test "-B poodle --detach bamboo main" twice.
>>
>> That seems to be added by mistake in 2/2, i.e. it's the existing test
>> you can see in the diff context, just added at the end.
>
> This is much clearer and more succinct. I've applied this to 2/2 for v4.

>
> > +test_expect_success '"add --orphan" fails if the branch already exists' '
> > +	test_when_finished "git branch -D existingbranch" &&
> > +	test_when_finished "git worktree remove -f -f orphandir" &&
> > +	git worktree add -b existingbranch orphandir main &&
> > +	test_must_fail git worktree add --orphan existingbranch orphandir2 &&
> > +	test ! -d orphandir2
>
> I'm not sure about "worktree" behavior, but maybe this "test ! -d" wants
> to be a "test_path_is_missing"?
>
> In general we try to test what a thing is, not what it isn't, in this
> case don't we fail to create the dir entirely? So "not exists" would
> cover it?

> On 22/11/15 10:08PM, Ævar Arnfjörð Bjarmason wrote:
> > > +   test_must_fail git worktree add --orphan existingbranch orphandir2 &&
> > > +   test ! -d orphandir2
> >
> > I'm not sure about "worktree" behavior, but maybe this "test ! -d" wants
> > to be a "test_path_is_missing"?
> >
> > In general we try to test what a thing is, not what it isn't, in this
> > case don't we fail to create the dir entirely? So "not exists" would
> > cover it?
>
> Ah yes that would be preferable. I've updated it for v4.
>
> This shows up in the file in a few other places in this file as well
> (from before this patch). Should I make the changes there as well and put
> those changes into an additional patch in this patchset?

> On Fri, Nov 18, 2022 at 8:44 PM Jacob Abel <jacobabel@nullpo.dev> wrote:
> > On 22/11/15 10:08PM, Ævar Arnfjörð Bjarmason wrote:
> > > > +   test_must_fail git worktree add --orphan existingbranch orphandir2 &&
> > > > +   test ! -d orphandir2
> > >
> > > I'm not sure about "worktree" behavior, but maybe this "test ! -d" wants
> > > to be a "test_path_is_missing"?
> > >
> > > In general we try to test what a thing is, not what it isn't, in this
> > > case don't we fail to create the dir entirely? So "not exists" would
> > > cover it?
> >
> > Ah yes that would be preferable. I've updated it for v4.
> >
> > This shows up in the file in a few other places in this file as well
> > (from before this patch). Should I make the changes there as well and put
> > those changes into an additional patch in this patchset?
>
> With my reviewer's hat on, my goal is to help the series land in
> Junio's tree, which means I'd like to see fewer changes with each
> iteration. Adding a new patch which is only tangentially related to
> what the series wants to achieve isn't a priority, and could end up
> delaying acceptance of the series if problems in the new patch end up
> requiring additional rerolls.
>
> So, yes, you could do that cleanup as a preparatory patch in the
> series if you want to tackle it. It would be an appropriate cleanup
> since you're working on code nearby. Or it could be done as a follow
> up to this series. Given how small the cleanup patch would likely be,
> it may not make a difference one way or the other, especially if the
> commit message explains the change well (for instance, by paraphrasing
> what Ævar said about "testing what a thing is, not what it isn't").

> +--orphan <new-branch>::
> +	With `add`, create a new orphan branch named `<new-branch>` in the new
> +	worktree. See `--orphan` in linkgit:git-switch[1] for details.
> +
>  --porcelain::
> ....
>  #define BUILTIN_WORKTREE_ADD_USAGE \
>  	N_("git worktree add [-f] [--detach] [--checkout] [--lock [--reason <string>]]\n" \
> -	   "                 [[-b | -B] <new-branch>] <path> [<commit-ish>]")
> +	   "                 [[-b | -B | --orphan] <new-branch>] <path> [<commit-ish>]")

> -	if (ac < 2 && !new_branch && !opts.detach) {
> +	/*
> +	 * As the orphan cannot be created until the contents of branch
> +	 * are staged, opts.orphan_branch should be treated as both a boolean
> +	 * indicating that `--orphan` was selected and as the name of the new
> +	 * orphan branch from this point on.
> +	 */

> +	if (opts.orphan_branch) {
> +		new_branch = opts.orphan_branch;
> +	}
> +
> +	if (ac < 2 && !new_branch && !opts.detach && !opts.orphan_branch) {

>
> On Thu, Nov 10 2022, Jacob Abel wrote:
>
> So, on a second read-through...
>
> >  'git worktree add' [-f] [--detach] [--checkout] [--lock [--reason <string>]]
> > -		   [[-b | -B] <new-branch>] <path> [<commit-ish>]
> > +		   [[-b | -B | --orphan] <new-branch>] <path> [<commit-ish>]
>
> This synopsis is now at least partially wrong, and ....
>
> > +--orphan <new-branch>::
> > +	With `add`, create a new orphan branch named `<new-branch>` in the new
> > +	worktree. See `--orphan` in linkgit:git-switch[1] for details.
> > +
> >  --porcelain::
> > ....
> >  #define BUILTIN_WORKTREE_ADD_USAGE \
> >  	N_("git worktree add [-f] [--detach] [--checkout] [--lock [--reason <string>]]\n" \
> > -	   "                 [[-b | -B] <new-branch>] <path> [<commit-ish>]")
> > +	   "                 [[-b | -B | --orphan] <new-branch>] <path> [<commit-ish>]")
>
>
> ...here we say the same, but surely it's only:
>
> 	git worktree add --orphan new-branch /tmp/orphan
>
> And not e.g.:
>
> 	git worktree add --orphan new-branch /tmp/orphan origin/next
>
> Or whatever, but it's incompatible with <commit-ish>. I think this on
> top should fix it up:
>
> 	diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt
> 	index 1310bfb564f..3afef985154 100644
> 	--- a/Documentation/git-worktree.txt
> 	+++ b/Documentation/git-worktree.txt
> 	@@ -10,7 +10,9 @@ SYNOPSIS
> 	 --------
> 	 [verse]
> 	 'git worktree add' [-f] [--detach] [--checkout] [--lock [--reason <string>]]
> 	-		   [[-b | -B | --orphan] <new-branch>] <path> [<commit-ish>]
> 	+		   [[-b | -B] <new-branch>] <path> [<commit-ish>]
> 	+'git worktree add' [-f] [--lock [--reason <string>]]
> 	+		   --orphan <new-branch> <path>
> 	 'git worktree list' [-v | --porcelain [-z]]
> 	 'git worktree lock' [--reason <string>] <worktree>
> 	 'git worktree move' <worktree> <new-path>
> 	diff --git a/builtin/worktree.c b/builtin/worktree.c
> 	index 71786b72f6b..2b811630b3a 100644
> 	--- a/builtin/worktree.c
> 	+++ b/builtin/worktree.c
> 	@@ -17,7 +17,10 @@
>
> 	 #define BUILTIN_WORKTREE_ADD_USAGE \
> 	 	N_("git worktree add [-f] [--detach] [--checkout] [--lock [--reason <string>]]\n" \
> 	-	   "                 [[-b | -B | --orphan] <new-branch>] <path> [<commit-ish>]")
> 	+	   "                 [[-b | -B] <new-branch>] <path> [<commit-ish>]"), \
> 	+	N_("git worktree add [-f] [--lock [--reason <string>]]\n" \
> 	+	   "                 --orphan <new-branch> <path>")
> 	+
> 	 #define BUILTIN_WORKTREE_LIST_USAGE \
> 	 	N_("git worktree list [-v | --porcelain [-z]]")
> 	 #define BUILTIN_WORKTREE_LOCK_USAGE \
> 	@@ -668,6 +671,9 @@ static int add(int ac, const char **av, const char *prefix)
> 	 	if (opts.orphan_branch && !opts.checkout)
> 	 		die(_("'%s' and '%s' cannot be used together"), "--orphan",
> 	 		    "--no-checkout");
> 	+	if (opts.orphan_branch && ac == 2)
> 	+		die(_("'%s' and '%s' cannot be used together"), "--orphan",
> 	+		    _("<commit-ish>"));
> 	 	if (lock_reason && !keep_locked)
> 	 		die(_("the option '%s' requires '%s'"), "--reason", "--lock");
> 	 	if (lock_reason)
> 	diff --git a/t/t2400-worktree-add.sh b/t/t2400-worktree-add.sh
> 	index 93c340f4aff..47461d02115 100755
> 	--- a/t/t2400-worktree-add.sh
> 	+++ b/t/t2400-worktree-add.sh
> 	@@ -326,6 +326,10 @@ test_expect_success '"add" --orphan/--no-checkout mutually exclusive' '
> 	 	test_must_fail git worktree add --orphan poodle --no-checkout bamboo
> 	 '
>
> 	+test_expect_success '"add" --orphan and <commit-ish> mutually exclusive' '
> 	+	test_must_fail git worktree add --orphan poodle bamboo main
> 	+'
> 	+
> 	 test_expect_success '"add" -B/--detach mutually exclusive' '
> 	 	test_must_fail git worktree add -B poodle --detach bamboo main
> 	 '

>
> > -	if (ac < 2 && !new_branch && !opts.detach) {
> > +	/*
> > +	 * As the orphan cannot be created until the contents of branch
> > +	 * are staged, opts.orphan_branch should be treated as both a boolean
> > +	 * indicating that `--orphan` was selected and as the name of the new
> > +	 * orphan branch from this point on.
> > +	 */
>
> I've re-read this a couple of times, and I honestly still don't see what
> point is trying to drive home.
>
> So, "--orphan" is an OPT_STRING(), so it always has a value:
>
> 	$ ./git worktree add --orphan
> 	error: option `orphan' requires a value
>
> But we init it to NULL, and above we just used it as a boolean *and*
> below.
>
> I.e. this comment would seem to me to fit with code where the reader
> might be surprised that we're using "opts.orphan_branch" as a string
> from then on, but we're just copying that to "new_branch", then we
> always use "opts.orphan_branch" as a boolean for the rest of the
> function.
>
> I may be missing something, but I think this would probably be better
> just without this comment. E.g. we use "--track", "--lock-reason"
> etc. in similar ways, and those don't have a comment like that.
>

>
> > +	if (opts.orphan_branch) {
> > +		new_branch = opts.orphan_branch;
> > +	}
> > +
> > +	if (ac < 2 && !new_branch && !opts.detach && !opts.orphan_branch) {
>
> In general we shouldn't combine random "if"'s just because a a
> sufficiently smart compiler could discover a way to reduce work.
>
> But in this case these seem to be inherently connected, we always want
> the not-DWIM behavior with "orphan", no?
>
> So shouldn't this just be:
>
> 	if (opts.orphan_branch) {
> 		...
> 	} else if (ac < 2 && !new_branch && !opts.detach) {
> 		....
> 	}
>
> ?

> On 22/11/15 11:09PM, Ævar Arnfjörð Bjarmason wrote:
>>
>> On Thu, Nov 10 2022, Jacob Abel wrote:
>>
>> So, on a second read-through...
>>
>> >  'git worktree add' [-f] [--detach] [--checkout] [--lock [--reason <string>]]
>> > -		   [[-b | -B] <new-branch>] <path> [<commit-ish>]
>> > +		   [[-b | -B | --orphan] <new-branch>] <path> [<commit-ish>]
>>
>> This synopsis is now at least partially wrong, and ....
>>
>> > +--orphan <new-branch>::
>> > +	With `add`, create a new orphan branch named `<new-branch>` in the new
>> > +	worktree. See `--orphan` in linkgit:git-switch[1] for details.
>> > +
>> >  --porcelain::
>> > ....
>> >  #define BUILTIN_WORKTREE_ADD_USAGE \
>> >  	N_("git worktree add [-f] [--detach] [--checkout] [--lock [--reason <string>]]\n" \
>> > -	   "                 [[-b | -B] <new-branch>] <path> [<commit-ish>]")
>> > +	   "                 [[-b | -B | --orphan] <new-branch>] <path> [<commit-ish>]")
>>
>>
>> ...here we say the same, but surely it's only:
>>
>> 	git worktree add --orphan new-branch /tmp/orphan
>>
>> And not e.g.:
>>
>> 	git worktree add --orphan new-branch /tmp/orphan origin/next
>>
>> Or whatever, but it's incompatible with <commit-ish>. I think this on
>> top should fix it up:
>>
>> 	diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt
>> 	index 1310bfb564f..3afef985154 100644
>> 	--- a/Documentation/git-worktree.txt
>> 	+++ b/Documentation/git-worktree.txt
>> 	@@ -10,7 +10,9 @@ SYNOPSIS
>> 	 --------
>> 	 [verse]
>> 	 'git worktree add' [-f] [--detach] [--checkout] [--lock [--reason <string>]]
>> 	-		   [[-b | -B | --orphan] <new-branch>] <path> [<commit-ish>]
>> 	+		   [[-b | -B] <new-branch>] <path> [<commit-ish>]
>> 	+'git worktree add' [-f] [--lock [--reason <string>]]
>> 	+		   --orphan <new-branch> <path>
>> 	 'git worktree list' [-v | --porcelain [-z]]
>> 	 'git worktree lock' [--reason <string>] <worktree>
>> 	 'git worktree move' <worktree> <new-path>
>> 	diff --git a/builtin/worktree.c b/builtin/worktree.c
>> 	index 71786b72f6b..2b811630b3a 100644
>> 	--- a/builtin/worktree.c
>> 	+++ b/builtin/worktree.c
>> 	@@ -17,7 +17,10 @@
>>
>> 	 #define BUILTIN_WORKTREE_ADD_USAGE \
>> 	 	N_("git worktree add [-f] [--detach] [--checkout] [--lock [--reason <string>]]\n" \
>> 	-	   "                 [[-b | -B | --orphan] <new-branch>] <path> [<commit-ish>]")
>> 	+	   "                 [[-b | -B] <new-branch>] <path> [<commit-ish>]"), \
>> 	+	N_("git worktree add [-f] [--lock [--reason <string>]]\n" \
>> 	+	   "                 --orphan <new-branch> <path>")
>> 	+
>> 	 #define BUILTIN_WORKTREE_LIST_USAGE \
>> 	 	N_("git worktree list [-v | --porcelain [-z]]")
>> 	 #define BUILTIN_WORKTREE_LOCK_USAGE \
>> 	@@ -668,6 +671,9 @@ static int add(int ac, const char **av, const char *prefix)
>> 	 	if (opts.orphan_branch && !opts.checkout)
>> 	 		die(_("'%s' and '%s' cannot be used together"), "--orphan",
>> 	 		    "--no-checkout");
>> 	+	if (opts.orphan_branch && ac == 2)
>> 	+		die(_("'%s' and '%s' cannot be used together"), "--orphan",
>> 	+		    _("<commit-ish>"));
>> 	 	if (lock_reason && !keep_locked)
>> 	 		die(_("the option '%s' requires '%s'"), "--reason", "--lock");
>> 	 	if (lock_reason)
>> 	diff --git a/t/t2400-worktree-add.sh b/t/t2400-worktree-add.sh
>> 	index 93c340f4aff..47461d02115 100755
>> 	--- a/t/t2400-worktree-add.sh
>> 	+++ b/t/t2400-worktree-add.sh
>> 	@@ -326,6 +326,10 @@ test_expect_success '"add" --orphan/--no-checkout mutually exclusive' '
>> 	 	test_must_fail git worktree add --orphan poodle --no-checkout bamboo
>> 	 '
>>
>> 	+test_expect_success '"add" --orphan and <commit-ish> mutually exclusive' '
>> 	+	test_must_fail git worktree add --orphan poodle bamboo main
>> 	+'
>> 	+
>> 	 test_expect_success '"add" -B/--detach mutually exclusive' '
>> 	 	test_must_fail git worktree add -B poodle --detach bamboo main
>> 	 '
>
> Yep, you are right. I applied the patch as part of this 2/2 patch and will
> include it in v4. When it comes to attribution, is there a preferred way to
> handle this?

>>
>> > -	if (ac < 2 && !new_branch && !opts.detach) {
>> > +	/*
>> > +	 * As the orphan cannot be created until the contents of branch
>> > +	 * are staged, opts.orphan_branch should be treated as both a boolean
>> > +	 * indicating that `--orphan` was selected and as the name of the new
>> > +	 * orphan branch from this point on.
>> > +	 */
>>
>> I've re-read this a couple of times, and I honestly still don't see what
>> point is trying to drive home.
>>
>> So, "--orphan" is an OPT_STRING(), so it always has a value:
>>
>> 	$ ./git worktree add --orphan
>> 	error: option `orphan' requires a value
>>
>> But we init it to NULL, and above we just used it as a boolean *and*
>> below.
>>
>> I.e. this comment would seem to me to fit with code where the reader
>> might be surprised that we're using "opts.orphan_branch" as a string
>> from then on, but we're just copying that to "new_branch", then we
>> always use "opts.orphan_branch" as a boolean for the rest of the
>> function.
>>
>> I may be missing something, but I think this would probably be better
>> just without this comment. E.g. we use "--track", "--lock-reason"
>> etc. in similar ways, and those don't have a comment like that.
>>
>
> Originally the new orphan branch's name was passed into
> `add_worktree(path, refname, opts)` via the `orphan_branch` field in `opts` and
> the branch which was to be checked out first(to mimic `git checkout --orphan`)
> was passed in via `refname`.
>
> Now that the behavior was changed to use `git switch`, that
> "checkout then make orphan" behavior was unneeded and `refname` also contains
> the name of the orphan branch.
>
> For `make_worktree_orphan(opts, child_env)` however since I used the same
> function signature as `checkout_worktree(opts, child_env)`, `refname` wasn't
> passed in and I used `opts->orphan_branch` to access the branch name from
> that scope.
>
> I can change `make_worktree_orphan(opts, child_env)` to
> `make_worktree_orphan(ref, opts, child_env)` instead and then `orphan_branch`
> would be able to be treated as a boolean like those other flags.

>>
>> > +	if (opts.orphan_branch) {
>> > +		new_branch = opts.orphan_branch;
>> > +	}
>> > +
>> > +	if (ac < 2 && !new_branch && !opts.detach && !opts.orphan_branch) {
>>
>> In general we shouldn't combine random "if"'s just because a a
>> sufficiently smart compiler could discover a way to reduce work.
>>
>> But in this case these seem to be inherently connected, we always want
>> the not-DWIM behavior with "orphan", no?
>>
>> So shouldn't this just be:
>>
>> 	if (opts.orphan_branch) {
>> 		...
>> 	} else if (ac < 2 && !new_branch && !opts.detach) {
>> 		....
>> 	}
>>
>> ?
>
> Yes. I've updated that for v4.

> While working with the worktree based git workflow, I realised that setting
> up a new git repository required switching between the traditional and
> worktree based workflows. Searching online I found a SO answer [1] which
> seemed to support this and which indicated that adding support for this should
> not be technically difficult.
>
>   * adding orphan branch functionality (as is present in `git-switch`)
>     to `git-worktree-add`

> Changes from v2:
>
>   * Changed orphan creation behavior to match `git switch --orphan` instead of
>     `git checkout --orphan` [2][3]. As a result `--orphan` no longer accepts a
>     `<commit-ish>` and creates the orphan branch with a clean working directory.

> On Thu, Nov 10, 2022 at 6:32 PM Jacob Abel <jacobabel@nullpo.dev> wrote:
>> While working with the worktree based git workflow, I realised that setting
>> up a new git repository required switching between the traditional and
>> worktree based workflows. Searching online I found a SO answer [1] which
>> seemed to support this and which indicated that adding support for this should
>> not be technically difficult.
>>
>>   * adding orphan branch functionality (as is present in `git-switch`)
>>     to `git-worktree-add`
>
> I haven't had a chance yet to read v3, but can we take a step back for
> a moment and look at this topic from a slightly different angle?
> Setting aside the value of adding --orphan to `git worktree add`
> (which, I'm perfectly fine with, as mentioned earlier), I have a
> question about whether the solution proposed by this series is the
> best we can do.
>
> As I understand it, the actual problem this series wants to solve is
> that it's not possible to create a new worktree from an empty bare
> repository; for instance:
>
>     % git init --bare foo.git
>     % git -C foo.git worktree add -b main bar
>     Preparing worktree (new branch 'main')
>     fatal: not a valid object name: 'HEAD'
>     %
>
> This series addresses that shortcoming by adding --orphan, so that the
> following works:
>
>     % git init --bare foo.git
>     % git -C foo.git worktree add --orphan main bar
>     Preparing worktree (new branch 'main')
>     %
>
> However, is this really the best and most user-friendly and most
> discoverable solution? Is it likely that users are somehow going to
> instinctively use --orphan when they see the "fatal: not a valid
> object name: 'HEAD'" error message?
>
> Wouldn't a better solution be to somehow fix `git worktree add -b
> <branch>` so that it just works rather than erroring out? I haven't
> delved into the implementation to determine if this is possible, but
> if it is, it seems a far superior "fix" for the problem shown above
> since it requires no extra effort on the user's part, and doesn't
> raise any discoverability red-flags (since nothing needs to be
> "discovered" if `-b <branch>` works as expected in the first place).
>
> If fixing `-b <branch>` to "just work" is possible, then --orphan is
> no longer a needed workaround but becomes "icing on the cake".

>
> On Tue, Nov 15 2022, Eric Sunshine wrote:
>
> > On Thu, Nov 10, 2022 at 6:32 PM Jacob Abel <jacobabel@nullpo.dev> wrote:
> >> While working with the worktree based git workflow, I realised that setting
> >> up a new git repository required switching between the traditional and
> >> worktree based workflows. Searching online I found a SO answer [1] which
> >> seemed to support this and which indicated that adding support for this should
> >> not be technically difficult.
> >>
> >>   * adding orphan branch functionality (as is present in `git-switch`)
> >>     to `git-worktree-add`
> >
> > I haven't had a chance yet to read v3, but can we take a step back for
> > a moment and look at this topic from a slightly different angle?
> > Setting aside the value of adding --orphan to `git worktree add`
> > (which, I'm perfectly fine with, as mentioned earlier), I have a
> > question about whether the solution proposed by this series is the
> > best we can do.
> >
> > As I understand it, the actual problem this series wants to solve is
> > that it's not possible to create a new worktree from an empty bare
> > repository; for instance:
> >
> >     % git init --bare foo.git
> >     % git -C foo.git worktree add -b main bar
> >     Preparing worktree (new branch 'main')
> >     fatal: not a valid object name: 'HEAD'
> >     %
> >
> > This series addresses that shortcoming by adding --orphan, so that the
> > following works:
> >
> >     % git init --bare foo.git
> >     % git -C foo.git worktree add --orphan main bar
> >     Preparing worktree (new branch 'main')
> >     %
> >
> > However, is this really the best and most user-friendly and most
> > discoverable solution? Is it likely that users are somehow going to
> > instinctively use --orphan when they see the "fatal: not a valid
> > object name: 'HEAD'" error message?
> >
> > Wouldn't a better solution be to somehow fix `git worktree add -b
> > <branch>` so that it just works rather than erroring out? I haven't
> > delved into the implementation to determine if this is possible, but
> > if it is, it seems a far superior "fix" for the problem shown above
> > since it requires no extra effort on the user's part, and doesn't
> > raise any discoverability red-flags (since nothing needs to be
> > "discovered" if `-b <branch>` works as expected in the first place).
> >
> > If fixing `-b <branch>` to "just work" is possible, then --orphan is
> > no longer a needed workaround but becomes "icing on the cake".
>
> That's a really good point, and we *could* "fix" that.
>
> But I don't see how to do it without overloading "-b" even further, in a
> way that some users either might not mean, or at least would be
> confusing.
>
> E.g. one script "manually clones" a repo because it does "git init",
> "git remote set-url", "git fetch" etc. Another one makes worktrees from
> those fresh checkouts once set up.
>
> If we "DWYM" here that second step will carry forward the bad state
> instead of erroring early.
>
> I haven't fully thought this throuh, so maybe it's fine, just
> wondering...
>
> ...an alternate way to perhaps to do this would be to detect this
> situation in add(), and emit an advise() telling the user that maybe
> they want to use "--orphan" for this?
>

> On 22/11/17 11:00AM, Ævar Arnfjörð Bjarmason wrote:
>>
>> On Tue, Nov 15 2022, Eric Sunshine wrote:
>>
>> > On Thu, Nov 10, 2022 at 6:32 PM Jacob Abel <jacobabel@nullpo.dev> wrote:
>> >> While working with the worktree based git workflow, I realised that setting
>> >> up a new git repository required switching between the traditional and
>> >> worktree based workflows. Searching online I found a SO answer [1] which
>> >> seemed to support this and which indicated that adding support for this should
>> >> not be technically difficult.
>> >>
>> >>   * adding orphan branch functionality (as is present in `git-switch`)
>> >>     to `git-worktree-add`
>> >
>> > I haven't had a chance yet to read v3, but can we take a step back for
>> > a moment and look at this topic from a slightly different angle?
>> > Setting aside the value of adding --orphan to `git worktree add`
>> > (which, I'm perfectly fine with, as mentioned earlier), I have a
>> > question about whether the solution proposed by this series is the
>> > best we can do.
>> >
>> > As I understand it, the actual problem this series wants to solve is
>> > that it's not possible to create a new worktree from an empty bare
>> > repository; for instance:
>> >
>> >     % git init --bare foo.git
>> >     % git -C foo.git worktree add -b main bar
>> >     Preparing worktree (new branch 'main')
>> >     fatal: not a valid object name: 'HEAD'
>> >     %
>> >
>> > This series addresses that shortcoming by adding --orphan, so that the
>> > following works:
>> >
>> >     % git init --bare foo.git
>> >     % git -C foo.git worktree add --orphan main bar
>> >     Preparing worktree (new branch 'main')
>> >     %
>> >
>> > However, is this really the best and most user-friendly and most
>> > discoverable solution? Is it likely that users are somehow going to
>> > instinctively use --orphan when they see the "fatal: not a valid
>> > object name: 'HEAD'" error message?
>> >
>> > Wouldn't a better solution be to somehow fix `git worktree add -b
>> > <branch>` so that it just works rather than erroring out? I haven't
>> > delved into the implementation to determine if this is possible, but
>> > if it is, it seems a far superior "fix" for the problem shown above
>> > since it requires no extra effort on the user's part, and doesn't
>> > raise any discoverability red-flags (since nothing needs to be
>> > "discovered" if `-b <branch>` works as expected in the first place).
>> >
>> > If fixing `-b <branch>` to "just work" is possible, then --orphan is
>> > no longer a needed workaround but becomes "icing on the cake".
>>
>> That's a really good point, and we *could* "fix" that.
>>
>> But I don't see how to do it without overloading "-b" even further, in a
>> way that some users either might not mean, or at least would be
>> confusing.
>>
>> E.g. one script "manually clones" a repo because it does "git init",
>> "git remote set-url", "git fetch" etc. Another one makes worktrees from
>> those fresh checkouts once set up.
>>
>> If we "DWYM" here that second step will carry forward the bad state
>> instead of erroring early.
>>
>> I haven't fully thought this throuh, so maybe it's fine, just
>> wondering...
>>
>> ...an alternate way to perhaps to do this would be to detect this
>> situation in add(), and emit an advise() telling the user that maybe
>> they want to use "--orphan" for this?
>>
>
> Prior to writing this patch, I tried to determine if there was a succinct way
> to make `-b` "just work" however I wasn't able to find one that wouldn't
> introduce unintuitive behavior. My conclusion was that it was probably best
> to break it out into a separate command as the other tools had.
>
> I'd support adding an `advise()` for at least the basic case where you try to
> create a worktree and no branches currently exist in the repository.
> i.e. something like this:
>
>     % git init --bare foo.git
>     % git -C foo.git branch --list
>
>     % git -C foo.git worktree add foobar/
>     hint: If you meant to create a new initial branch for this repository,
>     hint: e.g. 'main', you can do so using the --orphan option:
>     hint:
>     hint:   git worktree add --orphan main main/
>     hint:
>     fatal: invalid reference: 'foobar'
>
> and
>
>     % git init --bare foo.git
>     % git -C foo.git --no-pager branch --list
>
>     % git -C foo.git worktree add -b foobar foobardir/
>     hint: If you meant to create a new initial branch for this repository,
>     hint: e.g. 'main', you can do so using the --orphan option:
>     hint:
>     hint:   git worktree add --orphan main main/
>     hint:
>     fatal: invalid reference: 'foobar'

> but not in the following circumstances:
>
>     % git init --bare foo.git
>     % ...
>     % git -C foo.git --no-pager branch --list
>     + foo
>       bar
>     % git -C foo.git worktree add foobar/
>     Preparing worktree (new branch 'foobar')
>     HEAD is now at 319605f8f0 This is a commit message
>
> or
>
>     % git init --bare foo.git
>     % ...
>     % git -C foo.git --no-pager branch --list
>     + foo
>       bar
>     % git -C foo.git worktree add -b foobar foobardir/
>     Preparing worktree (new branch 'foobar)
>     HEAD is now at 319605f8f0 This is a commit message

> On Sat, Nov 19 2022, Jacob Abel wrote:
> > I'd support adding an `advise()` for at least the basic case where you try to
> > create a worktree and no branches currently exist in the repository.
> > i.e. something like this:
> >
> >     % git -C foo.git worktree add foobar/
> >     hint: If you meant to create a new initial branch for this repository,
> >     hint: e.g. 'main', you can do so using the --orphan option:
> >     hint:
> >     hint:   git worktree add --orphan main main/
> >     hint:
> >     fatal: invalid reference: 'foobar'
> > and
> >     % git -C foo.git worktree add -b foobar foobardir/
> >     hint: If you meant to create a new initial branch for this repository,
> >     hint: e.g. 'main', you can do so using the --orphan option:
> >     hint:
> >     hint:   git worktree add --orphan main main/
> >     hint:
> >     fatal: invalid reference: 'foobar'
>
> I think those would make sense, yes.

> > Would there be any other circumstances where we'd definitely want an `advise()`?
> > Generally I'd assume that outside of those two circumstances, most users will
> > rarely intend to make an orphan without already knowing they absolutely need to
> > make an orphan.
>
> I'm not familiar enough with the use-cases & workflow around "worktree"
> to say, sorry.

> On Sat, Nov 19, 2022 at 6:49 AM Ævar Arnfjörð Bjarmason
> <avarab@gmail.com> wrote:
> > On Sat, Nov 19 2022, Jacob Abel wrote:
> > > I'd support adding an `advise()` for at least the basic case where you try to
> > > create a worktree and no branches currently exist in the repository.
> > > i.e. something like this:
> > >
> > >     % git -C foo.git worktree add foobar/
> > >     hint: If you meant to create a new initial branch for this repository,
> > >     hint: e.g. 'main', you can do so using the --orphan option:
> > >     hint:
> > >     hint:   git worktree add --orphan main main/
> > >     hint:
> > >     fatal: invalid reference: 'foobar'
> > > and
> > >     % git -C foo.git worktree add -b foobar foobardir/
> > >     hint: If you meant to create a new initial branch for this repository,
> > >     hint: e.g. 'main', you can do so using the --orphan option:
> > >     hint:
> > >     hint:   git worktree add --orphan main main/
> > >     hint:
> > >     fatal: invalid reference: 'foobar'
> >
> > I think those would make sense, yes.
>
> Yes, this sort of advice could go a long way toward addressing my
> discoverability concerns. (I think, too, we should be able to
> dynamically customize the advice to mention "foobar" rather than
> "main" in order to more directly help the user.) Along with that,
> explaining this use-case in the git-worktree documentation would also
> be valuable for improving discoverability.

>
> Updating the commit message of patch [2/2] to explain this more fully
> would also be helpful for reviewers. It wasn't clear to me, for
> instance, during initial reviews and discussion that you were adding
> --orphan to make this use-case possible. Simply including in the
> commit message an example usage and associated error of the current
> implementation:
>
>     % git init --bare foo.git
>     % git -C foo.git worktree add -b main bar
>     Preparing worktree (new branch 'main')
>     fatal: not a valid object name: 'HEAD'
>     %
>
> would go a long way to help reviewers understand what this series is
> trying to achieve (at least it would have helped me).

>
> > > Would there be any other circumstances where we'd definitely want an `advise()`?
> > > Generally I'd assume that outside of those two circumstances, most users will
> > > rarely intend to make an orphan without already knowing they absolutely need to
> > > make an orphan.
> >
> > I'm not familiar enough with the use-cases & workflow around "worktree"
> > to say, sorry.
>
> It's probably fine to limit this advice to `git worktree add`,
> certainly for an initial implementation.

> On 22/11/22 12:16AM, Eric Sunshine wrote:
>> On Sat, Nov 19, 2022 at 6:49 AM Ævar Arnfjörð Bjarmason
>> <avarab@gmail.com> wrote:
>> > On Sat, Nov 19 2022, Jacob Abel wrote:
>> > > I'd support adding an `advise()` for at least the basic case where you try to
>> > > create a worktree and no branches currently exist in the repository.
>> > > i.e. something like this:
>> > >
>> > >     % git -C foo.git worktree add foobar/
>> > >     hint: If you meant to create a new initial branch for this repository,
>> > >     hint: e.g. 'main', you can do so using the --orphan option:
>> > >     hint:
>> > >     hint:   git worktree add --orphan main main/
>> > >     hint:
>> > >     fatal: invalid reference: 'foobar'
>> > > and
>> > >     % git -C foo.git worktree add -b foobar foobardir/
>> > >     hint: If you meant to create a new initial branch for this repository,
>> > >     hint: e.g. 'main', you can do so using the --orphan option:
>> > >     hint:
>> > >     hint:   git worktree add --orphan main main/
>> > >     hint:
>> > >     fatal: invalid reference: 'foobar'
>> >
>> > I think those would make sense, yes.
>>
>> Yes, this sort of advice could go a long way toward addressing my
>> discoverability concerns. (I think, too, we should be able to
>> dynamically customize the advice to mention "foobar" rather than
>> "main" in order to more directly help the user.) Along with that,
>> explaining this use-case in the git-worktree documentation would also
>> be valuable for improving discoverability.
>
> Perfect. I think I've got this working already on my end using more or less
> the following:
>
>     diff --git a/builtin/worktree.c b/builtin/worktree.c
>     index 71786b72f6..f65b63d9d2 100644
>     --- a/builtin/worktree.c
>     +++ b/builtin/worktree.c
>     @@ -736,7 +736,21 @@ static int add(int ac, const char **av, const char *prefix)
>         if (!opts.quiet)
>             print_preparing_worktree_line(opts.detach, branch, new_branch, !!new_branch_force);
>
>     -	if (new_branch && !opts.orphan_branch) {
>     +	if (opts.orphan_branch) {
>     +		branch = new_branch;
>     +	} else if (!lookup_commit_reference_by_name("head")) {
>     +		/*
>     +		 * if head does not reference a valid commit, only worktrees
>     +		 * based on orphan branches can be created.
>     +		 */
>     +		advise("if you meant to create a new orphan branch for this repository,\n"
>     +			 "e.g. '%s', you can do so using the --orphan option:\n"
>     +			 "\n"
>     +			 "	git worktree add --orphan %s %s\n"
>     +			 "\n",
>     +			 new_branch, new_branch, path);

>
> On Tue, Nov 22 2022, Jacob Abel wrote:
>
> > On 22/11/22 12:16AM, Eric Sunshine wrote:
> >> On Sat, Nov 19, 2022 at 6:49 AM Ævar Arnfjörð Bjarmason
> >> <avarab@gmail.com> wrote:
> >> > On Sat, Nov 19 2022, Jacob Abel wrote:
> >> > > I'd support adding an `advise()` for at least the basic case where you try to
> >> > > create a worktree and no branches currently exist in the repository.
> >> > > i.e. something like this:
> >> > >
> >> > >     % git -C foo.git worktree add foobar/
> >> > >     hint: If you meant to create a new initial branch for this repository,
> >> > >     hint: e.g. 'main', you can do so using the --orphan option:
> >> > >     hint:
> >> > >     hint:   git worktree add --orphan main main/
> >> > >     hint:
> >> > >     fatal: invalid reference: 'foobar'
> >> > > and
> >> > >     % git -C foo.git worktree add -b foobar foobardir/
> >> > >     hint: If you meant to create a new initial branch for this repository,
> >> > >     hint: e.g. 'main', you can do so using the --orphan option:
> >> > >     hint:
> >> > >     hint:   git worktree add --orphan main main/
> >> > >     hint:
> >> > >     fatal: invalid reference: 'foobar'
> >> >
> >> > I think those would make sense, yes.
> >>
> >> Yes, this sort of advice could go a long way toward addressing my
> >> discoverability concerns. (I think, too, we should be able to
> >> dynamically customize the advice to mention "foobar" rather than
> >> "main" in order to more directly help the user.) Along with that,
> >> explaining this use-case in the git-worktree documentation would also
> >> be valuable for improving discoverability.
> >
> > Perfect. I think I've got this working already on my end using more or less
> > the following:
> >
> >     diff --git a/builtin/worktree.c b/builtin/worktree.c
> >     index 71786b72f6..f65b63d9d2 100644
> >     --- a/builtin/worktree.c
> >     +++ b/builtin/worktree.c
> >     @@ -736,7 +736,21 @@ static int add(int ac, const char **av, const char *prefix)
> >         if (!opts.quiet)
> >             print_preparing_worktree_line(opts.detach, branch, new_branch, !!new_branch_force);
> >
> >     -	if (new_branch && !opts.orphan_branch) {
> >     +	if (opts.orphan_branch) {
> >     +		branch = new_branch;
> >     +	} else if (!lookup_commit_reference_by_name("head")) {
> >     +		/*
> >     +		 * if head does not reference a valid commit, only worktrees
> >     +		 * based on orphan branches can be created.
> >     +		 */
> >     +		advise("if you meant to create a new orphan branch for this repository,\n"
> >     +			 "e.g. '%s', you can do so using the --orphan option:\n"
> >     +			 "\n"
> >     +			 "	git worktree add --orphan %s %s\n"
> >     +			 "\n",
> >     +			 new_branch, new_branch, path);
>
> We don't consistently check for this, unfortunately (but I have some
> local patches for it), but to add an advice you should:
>
>  * Add it to Documentation/config/advice.txt (in sorted order)
>  * Add the corresponding enum to advice.h
>  * And to the advice.c listing
>  * Then use advise_if_enabled(<that new enum>, ...) in cases such as this one.
>  * End your message with a suggstion about how to disable the advice:
>    git grep -W -F 'git config advice.' -- '*.c'
>
> That's rather tedious, sorry, but that's the extent of the current
> boilerplate...

> On 22/11/22 12:16AM, Eric Sunshine wrote:
> > On Sat, Nov 19, 2022 at 6:49 AM Ævar Arnfjörð Bjarmason
> > <avarab@gmail.com> wrote:
> > > On Sat, Nov 19 2022, Jacob Abel wrote:
> > > > I'd support adding an `advise()` for at least the basic case where you try to
> > > > create a worktree and no branches currently exist in the repository.
> > > > i.e. something like this:
> > > >
> > > >     % git -C foo.git worktree add foobar/
> > > >     hint: If you meant to create a new initial branch for this repository,
> > > >     hint: e.g. 'main', you can do so using the --orphan option:
> > > >     hint:
> > > >     hint:   git worktree add --orphan main main/
> > > >     hint:
> > > >     fatal: invalid reference: 'foobar'
> > > > and
> > > >     % git -C foo.git worktree add -b foobar foobardir/
> > > >     hint: If you meant to create a new initial branch for this repository,
> > > >     hint: e.g. 'main', you can do so using the --orphan option:
> > > >     hint:
> > > >     hint:   git worktree add --orphan main main/
> > > >     hint:
> > > >     fatal: invalid reference: 'foobar'
> > >
> > > I think those would make sense, yes.
> >
> > Yes, this sort of advice could go a long way toward addressing my
> > discoverability concerns. (I think, too, we should be able to
> > dynamically customize the advice to mention "foobar" rather than
> > "main" in order to more directly help the user.) Along with that,
> > explaining this use-case in the git-worktree documentation would also
> > be valuable for improving discoverability.
> 
> Perfect. I think I've got this working already on my end using more or less
> the following:
> 
>     diff --git a/builtin/worktree.c b/builtin/worktree.c
>     index 71786b72f6..f65b63d9d2 100644
>     --- a/builtin/worktree.c
>     +++ b/builtin/worktree.c
>     @@ -736,7 +736,21 @@ static int add(int ac, const char **av, const char *prefix)
>         if (!opts.quiet)
>             print_preparing_worktree_line(opts.detach, branch, new_branch, !!new_branch_force);
> 
>     -	if (new_branch && !opts.orphan_branch) {
>     +	if (opts.orphan_branch) {
>     +		branch = new_branch;
>     +	} else if (!lookup_commit_reference_by_name("head")) {

> On 22-nov-2022 23:26:57, Jacob Abel wrote:
> > On 22/11/22 12:16AM, Eric Sunshine wrote:
> > > On Sat, Nov 19, 2022 at 6:49 AM Ævar Arnfjörð Bjarmason
> > > <avarab@gmail.com> wrote:
> > > > On Sat, Nov 19 2022, Jacob Abel wrote:
> > > > > I'd support adding an `advise()` for at least the basic case where you try to
> > > > > create a worktree and no branches currently exist in the repository.
> > > > > i.e. something like this:
> > > > >
> > > > >     % git -C foo.git worktree add foobar/
> > > > >     hint: If you meant to create a new initial branch for this repository,
> > > > >     hint: e.g. 'main', you can do so using the --orphan option:
> > > > >     hint:
> > > > >     hint:   git worktree add --orphan main main/
> > > > >     hint:
> > > > >     fatal: invalid reference: 'foobar'
> > > > > and
> > > > >     % git -C foo.git worktree add -b foobar foobardir/
> > > > >     hint: If you meant to create a new initial branch for this repository,
> > > > >     hint: e.g. 'main', you can do so using the --orphan option:
> > > > >     hint:
> > > > >     hint:   git worktree add --orphan main main/
> > > > >     hint:
> > > > >     fatal: invalid reference: 'foobar'
> > > >
> > > > I think those would make sense, yes.
> > >
> > > Yes, this sort of advice could go a long way toward addressing my
> > > discoverability concerns. (I think, too, we should be able to
> > > dynamically customize the advice to mention "foobar" rather than
> > > "main" in order to more directly help the user.) Along with that,
> > > explaining this use-case in the git-worktree documentation would also
> > > be valuable for improving discoverability.
> >
> > Perfect. I think I've got this working already on my end using more or less
> > the following:
> >
> >     diff --git a/builtin/worktree.c b/builtin/worktree.c
> >     index 71786b72f6..f65b63d9d2 100644
> >     --- a/builtin/worktree.c
> >     +++ b/builtin/worktree.c
> >     @@ -736,7 +736,21 @@ static int add(int ac, const char **av, const char *prefix)
> >         if (!opts.quiet)
> >             print_preparing_worktree_line(opts.detach, branch, new_branch, !!new_branch_force);
> >
> >     -	if (new_branch && !opts.orphan_branch) {
> >     +	if (opts.orphan_branch) {
> >     +		branch = new_branch;
> >     +	} else if (!lookup_commit_reference_by_name("head")) {
>
> I haven't read the full thread and sorry to enter this way in the
> conversation, but this line got my attention.

>
> Thank you for working on this, this is a thing that has hit me several
> times.
>
> The first impression got me thinking.. Why do we need this advise?
> Why not make the orphan branch right away? And why the argument for the
> --orphan option?

>
> This needs more work, but something like this:
>
> --- >8 ---
>
> diff --git a/builtin/worktree.c b/builtin/worktree.c
> index d774ff192a..1ea8d05c2f 100644
> --- a/builtin/worktree.c
> +++ b/builtin/worktree.c
> @@ -406,7 +406,7 @@ static int add_worktree(const char *path, const char *refname,
>
>  	/* is 'refname' a branch or commit? */
>  	if (!opts->detach && !strbuf_check_branch_ref(&symref, refname) &&
> -	    ref_exists(symref.buf)) {
> +	    (opts->orphan_branch || ref_exists(symref.buf))) {
>  		is_branch = 1;
>  		if (!opts->force)
>  			die_if_checked_out(symref.buf, 0);
> @@ -738,18 +738,8 @@ static int add(int ac, const char **av, const char *prefix)
>
>  	if (opts.orphan_branch) {
>  		branch = new_branch;
> -	} else if (!lookup_commit_reference_by_name("head")) {
> -		/*
> -		 * if head does not reference a valid commit, only worktrees
> -		 * based on orphan branches can be created.
> -		 */
> -		advise("if you meant to create a new orphan branch for this repository,\n"
> -			 "e.g. '%s', you can do so using the --orphan option:\n"
> -			 "\n"
> -			 "	git worktree add --orphan %s %s\n"
> -			 "\n",
> -			 new_branch, new_branch, path);
> -		die(_("invalid reference: %s"), new_branch);
> +	} else if (new_branch && !lookup_commit_reference_by_name("HEAD")) {
> +		branch = opts.orphan_branch = new_branch;
>  	} else if (new_branch) {
>  		struct child_process cp = CHILD_PROCESS_INIT;
>  		cp.git_cmd = 1;

> On 22/11/23 03:43AM, Rubén Justo wrote:
> > On 22-nov-2022 23:26:57, Jacob Abel wrote:
> > > On 22/11/22 12:16AM, Eric Sunshine wrote:
> > > > On Sat, Nov 19, 2022 at 6:49 AM Ævar Arnfjörð Bjarmason
> > > > <avarab@gmail.com> wrote:
> > > > > On Sat, Nov 19 2022, Jacob Abel wrote:
> > > > > > I'd support adding an `advise()` for at least the basic case where you try to
> > > > > > create a worktree and no branches currently exist in the repository.
> > > > > > i.e. something like this:
> > > > > >
> > > > > >     % git -C foo.git worktree add foobar/
> > > > > >     hint: If you meant to create a new initial branch for this repository,
> > > > > >     hint: e.g. 'main', you can do so using the --orphan option:
> > > > > >     hint:
> > > > > >     hint:   git worktree add --orphan main main/
> > > > > >     hint:
> > > > > >     fatal: invalid reference: 'foobar'
> > > > > > and
> > > > > >     % git -C foo.git worktree add -b foobar foobardir/
> > > > > >     hint: If you meant to create a new initial branch for this repository,
> > > > > >     hint: e.g. 'main', you can do so using the --orphan option:
> > > > > >     hint:
> > > > > >     hint:   git worktree add --orphan main main/
> > > > > >     hint:
> > > > > >     fatal: invalid reference: 'foobar'
> > > > >
> > > > > I think those would make sense, yes.
> > > >
> > > > Yes, this sort of advice could go a long way toward addressing my
> > > > discoverability concerns. (I think, too, we should be able to
> > > > dynamically customize the advice to mention "foobar" rather than
> > > > "main" in order to more directly help the user.) Along with that,
> > > > explaining this use-case in the git-worktree documentation would also
> > > > be valuable for improving discoverability.
> > >
> > > Perfect. I think I've got this working already on my end using more or less
> > > the following:
> > >
> > >     diff --git a/builtin/worktree.c b/builtin/worktree.c
> > >     index 71786b72f6..f65b63d9d2 100644
> > >     --- a/builtin/worktree.c
> > >     +++ b/builtin/worktree.c
> > >     @@ -736,7 +736,21 @@ static int add(int ac, const char **av, const char *prefix)
> > >         if (!opts.quiet)
> > >             print_preparing_worktree_line(opts.detach, branch, new_branch, !!new_branch_force);
> > >
> > >     -	if (new_branch && !opts.orphan_branch) {
> > >     +	if (opts.orphan_branch) {
> > >     +		branch = new_branch;
> > >     +	} else if (!lookup_commit_reference_by_name("head")) {
> >
> > I haven't read the full thread and sorry to enter this way in the
> > conversation, but this line got my attention.
> 
> No worries. It's always nice to have more eyes to catch mistakes.
> 
> > This needs to be "HEAD", in capital letters.
> 
> Ah yes. I wasn't paying attention when I copied it into my MUA and must have
> accidentally typed `ggvGu` instead of `ggvGy` and lowercased it before I copied
> it (thanks vim/user error). It should be:
> 
>     diff --git a/builtin/worktree.c b/builtin/worktree.c
>     index 71786b72f6..f65b63d9d2 100644
>     --- a/builtin/worktree.c
>     +++ b/builtin/worktree.c
>     @@ -736,7 +736,21 @@ static int add(int ac, const char **av, const char *prefix)
>         if (!opts.quiet)
>             print_preparing_worktree_line(opts.detach, branch, new_branch, !!new_branch_force);
> 
>     -	if (new_branch && !opts.orphan_branch) {
>     +	if (opts.orphan_branch) {
>     +		branch = new_branch;
>     +	} else if (!lookup_commit_reference_by_name("HEAD")) {
>     +		/*
>     +		 * If HEAD does not reference a valid commit, only worktrees
>     +		 * based on orphan branches can be created.
>     +		 */
>     +		advise("If you meant to create a new orphan branch for this repository,\n"
>     +			 "e.g. '%s', you can do so using the --orphan option:\n"
>     +			 "\n"
>     +			 "	git worktree add --orphan %s %s\n"
>     +			 "\n",
>     +			 new_branch, new_branch, path);
>     +		die(_("invalid reference: %s"), new_branch);
>     +	} else if (new_branch) {
>             struct child_process cp = CHILD_PROCESS_INIT;
>             cp.git_cmd = 1;
>             strvec_push(&cp.args, "branch");
> 
> 
> >
> > Thank you for working on this, this is a thing that has hit me several
> > times.
> >
> > The first impression got me thinking.. Why do we need this advise?
> > Why not make the orphan branch right away? And why the argument for the
> > --orphan option?
> 
> I went into my concerns with further overloading `worktree add -b/-B` and
> `worktree add` (DWYM) over on the other side of this thread [1]. I won't echo
> it all here but I wanted to mention a few things.
> 
> As for why we want the advise, by not short circuiting with the advise and
> instead just trying to DWYM, we can catch the following edge case:
> 
> A user less well acquainted with git tries out worktrees on a new project (no
> branches). They create multiple worktrees and since there are no branches, they
> are all orphans. Unless they've read the docs, they are now accustomed to this
> "new worktrees have no history" behavior. Then they make a commit on one of the
> orphans and the behavior changes and all new worktrees derive from that branch
> unless `git worktree add` is run from inside another worktree with a non-orphan
> branch.
> 
> There's more to it in the other thread but it gets kinda messy for the user if
> they walk off the well trodden path inadvertently. I'd like to avoid that all
> together where possible.
> 
> As for the argument, the reason is so that the syntax matches
> `git switch --orphan <new_branch>` (and the `git checkout` variant).
> 
> > I like what this new flag allows: make a new orphan branch when we
> > are in any branch.  But if we are already in an orphan branch (like the
> > initial) what's the user's expectation?
> 
> Like mentioned above (and in [1]), further overloading DWYM and `-b` impacts the
> already somewhat complex/unclear expectations for `git worktree add`.
> 
> When using the flag and not adding to `-b` and DWYM, we can short circuit this
> confusion for the most part by requiring the user to explicitly request
> `--orphan`.
> 
> As for creating a new orphan in a repo with existing branches but from a
> worktree containing an orphan branch, that fails cleanly as shown below:
> 
>     # in worktree with orphan branch
>     % git worktree add -b foobar ../foobar
>     Preparing worktree (new branch 'foobar')
>     fatal: invalid reference: foobar
> 
> and in the next revision should fail with the following:
> 
>     # in worktree with orphan branch
>     % git worktree add -b foobar ../foobar
>     Preparing worktree (new branch 'foobar')
>     hint: If you meant to create a new orphan branch for this repository,
>     hint: e.g. 'foobar', you can do so using the --orphan option:
>     hint:
>     hint:   git worktree add --orphan foobar ../foobar/
>     hint:
>     fatal: invalid reference: foobar
> 
> > Maybe we can use the new flag to indicate that the user unconditionally
> > wants an orphan branch, and use the rest of the arguments as they are,
> > '-b' included.
> 
> I wouldn't necessarily be opposed to this however I do think changing it from
> `--orphan <new_branch>` to `--orphan -b <new_branch>` would be a departure from
> the syntax used in `git switch` and `git checkout` and that may make it harder
> for users already familar with those other commands.
> 

> On 22/11/17 11:00AM, Ævar Arnfjörð Bjarmason wrote:
>>
>> On Tue, Nov 15 2022, Eric Sunshine wrote:
>>
>>> On Thu, Nov 10, 2022 at 6:32 PM Jacob Abel <jacobabel@nullpo.dev> wrote:
>>>> While working with the worktree based git workflow, I realised that setting
>>>> up a new git repository required switching between the traditional and
>>>> worktree based workflows. Searching online I found a SO answer [1] which
>>>> seemed to support this and which indicated that adding support for this should
>>>> not be technically difficult.
>>>>
>>>>    * adding orphan branch functionality (as is present in `git-switch`)
>>>>      to `git-worktree-add`
>>>
>>> I haven't had a chance yet to read v3, but can we take a step back for
>>> a moment and look at this topic from a slightly different angle?
>>> Setting aside the value of adding --orphan to `git worktree add`
>>> (which, I'm perfectly fine with, as mentioned earlier), I have a
>>> question about whether the solution proposed by this series is the
>>> best we can do.
>>>
>>> As I understand it, the actual problem this series wants to solve is
>>> that it's not possible to create a new worktree from an empty bare
>>> repository; for instance:
>>>
>>>      % git init --bare foo.git
>>>      % git -C foo.git worktree add -b main bar
>>>      Preparing worktree (new branch 'main')
>>>      fatal: not a valid object name: 'HEAD'
>>>      %
>>>
>>> This series addresses that shortcoming by adding --orphan, so that the
>>> following works:
>>>
>>>      % git init --bare foo.git
>>>      % git -C foo.git worktree add --orphan main bar
>>>      Preparing worktree (new branch 'main')
>>>      %
>>>
>>> However, is this really the best and most user-friendly and most
>>> discoverable solution? Is it likely that users are somehow going to
>>> instinctively use --orphan when they see the "fatal: not a valid
>>> object name: 'HEAD'" error message?
>>>
>>> Wouldn't a better solution be to somehow fix `git worktree add -b
>>> <branch>` so that it just works rather than erroring out? I haven't
>>> delved into the implementation to determine if this is possible, but
>>> if it is, it seems a far superior "fix" for the problem shown above
>>> since it requires no extra effort on the user's part, and doesn't
>>> raise any discoverability red-flags (since nothing needs to be
>>> "discovered" if `-b <branch>` works as expected in the first place).
>>>
>>> If fixing `-b <branch>` to "just work" is possible, then --orphan is
>>> no longer a needed workaround but becomes "icing on the cake".
>>
>> That's a really good point, and we *could* "fix" that.
>>
>> But I don't see how to do it without overloading "-b" even further, in a
>> way that some users either might not mean, or at least would be
>> confusing.
>>
>> E.g. one script "manually clones" a repo because it does "git init",
>> "git remote set-url", "git fetch" etc. Another one makes worktrees from
>> those fresh checkouts once set up.
>>
>> If we "DWYM" here that second step will carry forward the bad state
>> instead of erroring early.

>> I haven't fully thought this throuh, so maybe it's fine, just
>> wondering...
>>
>> ...an alternate way to perhaps to do this would be to detect this
>> situation in add(), and emit an advise() telling the user that maybe
>> they want to use "--orphan" for this?
>>
> 
> Prior to writing this patch, I tried to determine if there was a succinct way
> to make `-b` "just work" however I wasn't able to find one that wouldn't
> introduce unintuitive behavior.

> My conclusion was that it was probably best
> to break it out into a separate command as the other tools had.
> 
> I'd support adding an `advise()` for at least the basic case where you try to
> create a worktree and no branches currently exist in the repository.
> i.e. something like this:
> 
>      % git init --bare foo.git
>      % git -C foo.git branch --list
> 
>      % git -C foo.git worktree add foobar/
>      hint: If you meant to create a new initial branch for this repository,
>      hint: e.g. 'main', you can do so using the --orphan option:
>      hint:
>      hint:   git worktree add --orphan main main/
>      hint:
>      fatal: invalid reference: 'foobar'
> 
> and
> 
>      % git init --bare foo.git
>      % git -C foo.git --no-pager branch --list
> 
>      % git -C foo.git worktree add -b foobar foobardir/
>      hint: If you meant to create a new initial branch for this repository,
>      hint: e.g. 'main', you can do so using the --orphan option:
>      hint:
>      hint:   git worktree add --orphan main main/
>      hint:
>      fatal: invalid reference: 'foobar'
> 
> but not in the following circumstances:
> 
>      % git init --bare foo.git
>      % ...
>      % git -C foo.git --no-pager branch --list
>      + foo
>        bar
>      % git -C foo.git worktree add foobar/
>      Preparing worktree (new branch 'foobar')
>      HEAD is now at 319605f8f0 This is a commit message
> 
> or
> 
>      % git init --bare foo.git
>      % ...
>      % git -C foo.git --no-pager branch --list
>      + foo
>        bar
>      % git -C foo.git worktree add -b foobar foobardir/
>      Preparing worktree (new branch 'foobar)
>      HEAD is now at 319605f8f0 This is a commit message
> 
> Would there be any other circumstances where we'd definitely want an `advise()`?
> Generally I'd assume that outside of those two circumstances, most users will
> rarely intend to make an orphan without already knowing they absolutely need to
> make an orphan.

> On 19/11/2022 03:47, Jacob Abel wrote:
> > On 22/11/17 11:00AM, Ævar Arnfjörð Bjarmason wrote:
> >>
> >> On Tue, Nov 15 2022, Eric Sunshine wrote:
> >>
> >>> On Thu, Nov 10, 2022 at 6:32 PM Jacob Abel <jacobabel@nullpo.dev> wrote:
> >>>> While working with the worktree based git workflow, I realised that setting
> >>>> up a new git repository required switching between the traditional and
> >>>> worktree based workflows. Searching online I found a SO answer [1] which
> >>>> seemed to support this and which indicated that adding support for this should
> >>>> not be technically difficult.
> >>>>
> >>>>    * adding orphan branch functionality (as is present in `git-switch`)
> >>>>      to `git-worktree-add`
> >>>
> >>> I haven't had a chance yet to read v3, but can we take a step back for
> >>> a moment and look at this topic from a slightly different angle?
> >>> Setting aside the value of adding --orphan to `git worktree add`
> >>> (which, I'm perfectly fine with, as mentioned earlier), I have a
> >>> question about whether the solution proposed by this series is the
> >>> best we can do.
> >>>
> >>> As I understand it, the actual problem this series wants to solve is
> >>> that it's not possible to create a new worktree from an empty bare
> >>> repository; for instance:
> >>>
> >>>      % git init --bare foo.git
> >>>      % git -C foo.git worktree add -b main bar
> >>>      Preparing worktree (new branch 'main')
> >>>      fatal: not a valid object name: 'HEAD'
> >>>      %
> >>>
> >>> This series addresses that shortcoming by adding --orphan, so that the
> >>> following works:
> >>>
> >>>      % git init --bare foo.git
> >>>      % git -C foo.git worktree add --orphan main bar
> >>>      Preparing worktree (new branch 'main')
> >>>      %
> >>>
> >>> However, is this really the best and most user-friendly and most
> >>> discoverable solution? Is it likely that users are somehow going to
> >>> instinctively use --orphan when they see the "fatal: not a valid
> >>> object name: 'HEAD'" error message?
> >>>
> >>> Wouldn't a better solution be to somehow fix `git worktree add -b
> >>> <branch>` so that it just works rather than erroring out? I haven't
> >>> delved into the implementation to determine if this is possible, but
> >>> if it is, it seems a far superior "fix" for the problem shown above
> >>> since it requires no extra effort on the user's part, and doesn't
> >>> raise any discoverability red-flags (since nothing needs to be
> >>> "discovered" if `-b <branch>` works as expected in the first place).
> >>>
> >>> If fixing `-b <branch>` to "just work" is possible, then --orphan is
> >>> no longer a needed workaround but becomes "icing on the cake".
> >>
> >> That's a really good point, and we *could* "fix" that.
> >>
> >> But I don't see how to do it without overloading "-b" even further, in a
> >> way that some users either might not mean, or at least would be
> >> confusing.
> >>
> >> E.g. one script "manually clones" a repo because it does "git init",
> >> "git remote set-url", "git fetch" etc. Another one makes worktrees from
> >> those fresh checkouts once set up.
> >>
> >> If we "DWYM" here that second step will carry forward the bad state
> >> instead of erroring early.
>
> Wouldn't the first script error out if there was a problem?
>
> >> I haven't fully thought this throuh, so maybe it's fine, just
> >> wondering...
> >>
> >> ...an alternate way to perhaps to do this would be to detect this
> >> situation in add(), and emit an advise() telling the user that maybe
> >> they want to use "--orphan" for this?
> >>
> >
> > Prior to writing this patch, I tried to determine if there was a succinct way
> > to make `-b` "just work" however I wasn't able to find one that wouldn't
> > introduce unintuitive behavior.
>
> Can you say a bit more about what the unintuitive behavior was? As I
> understand it the problem is that "git branch" errors out when HEAD is a
> symbolic ref pointing to a ref that does not exist. I think we can use
> read_ref() to check for that before running "git branch" and act
> accordingly. We might want to check if HEAD matches init.defaultBranch
> and only do an orphan checkout in the new worktree in that case.

>
> > My conclusion was that it was probably best
> > to break it out into a separate command as the other tools had.
> >
> > I'd support adding an `advise()` for at least the basic case where you try to
> > create a worktree and no branches currently exist in the repository.
> > i.e. something like this:
> >
> >      % git init --bare foo.git
> >      % git -C foo.git branch --list
> >
> >      % git -C foo.git worktree add foobar/
> >      hint: If you meant to create a new initial branch for this repository,
> >      hint: e.g. 'main', you can do so using the --orphan option:
> >      hint:
> >      hint:   git worktree add --orphan main main/
> >      hint:
> >      fatal: invalid reference: 'foobar'
> >
> > and
> >
> >      % git init --bare foo.git
> >      % git -C foo.git --no-pager branch --list
> >
> >      % git -C foo.git worktree add -b foobar foobardir/
> >      hint: If you meant to create a new initial branch for this repository,
> >      hint: e.g. 'main', you can do so using the --orphan option:
> >      hint:
> >      hint:   git worktree add --orphan main main/
> >      hint:
> >      fatal: invalid reference: 'foobar'
> >
> > but not in the following circumstances:
> >
> >      % git init --bare foo.git
> >      % ...
> >      % git -C foo.git --no-pager branch --list
> >      + foo
> >        bar
> >      % git -C foo.git worktree add foobar/
> >      Preparing worktree (new branch 'foobar')
> >      HEAD is now at 319605f8f0 This is a commit message
> >
> > or
> >
> >      % git init --bare foo.git
> >      % ...
> >      % git -C foo.git --no-pager branch --list
> >      + foo
> >        bar
> >      % git -C foo.git worktree add -b foobar foobardir/
> >      Preparing worktree (new branch 'foobar)
> >      HEAD is now at 319605f8f0 This is a commit message
> >
> > Would there be any other circumstances where we'd definitely want an `advise()`?
> > Generally I'd assume that outside of those two circumstances, most users will
> > rarely intend to make an orphan without already knowing they absolutely need to
> > make an orphan.
>
> I don't think it matters if the repository is bare so I think it would
> be good to advise() on
>
> 	% git init foo
> 	% git -C foo worktree add bar
>
> Best Wishes
>
> Phillip

> +}
> +
>  static int add_worktree(const char *path, const char *refname,
>  			const struct add_opts *opts)
>  {
> @@ -393,8 +415,9 @@ static int add_worktree(const char *path, const char *refname,
>  			die_if_checked_out(symref.buf, 0);
>  	}
>  	commit = lookup_commit_reference_by_name(refname);
> -	if (!commit)
> +	if (!commit && !opts->orphan) {
>  		die(_("invalid reference: %s"), refname);
> +	}

>
>  	name = worktree_basename(path, &len);
>  	strbuf_add(&sb, name, path + len - name);
> @@ -482,10 +505,10 @@ static int add_worktree(const char *path, const char *refname,
>  	strvec_pushf(&child_env, "%s=%s", GIT_WORK_TREE_ENVIRONMENT, path);
>  	cp.git_cmd = 1;
>
> -	if (!is_branch)
> +	if (!is_branch && commit) {
>  		strvec_pushl(&cp.args, "update-ref", "HEAD",
>  			     oid_to_hex(&commit->object.oid), NULL);
> -	else {
> +	} else {

> +	/*
> +	 * When creating a new branch, new_branch now contains the branch to
> +	 * create.
> +	 *
> +	 * Past this point, new_branch_force can be treated solely as a
> +	 * boolean flag to indicate whether `-B` was selected.
> +	 */
>  	if (new_branch_force) {
>  		struct strbuf symref = STRBUF_INIT;
>

> -	if (new_branch) {
> +	if (opts.orphan) {
> +		branch = new_branch;
> +	} else if (!lookup_commit_reference_by_name(branch)) {
> +		/*
> +		 * If `branch` does not reference a valid commit, a new
> +		 * worktree (and/or branch) cannot be created based off of it.
> +		 */

> -test_expect_success '"add" -b/-B mutually exclusive' '
> -	test_must_fail git worktree add -b poodle -B poodle bamboo main
> -'
> -
> -test_expect_success '"add" -b/--detach mutually exclusive' '
> -	test_must_fail git worktree add -b poodle --detach bamboo main
> -'
> +# Helper function to test mutually exclusive options.
> +test_wt_add_excl() {
> +	local opts="$@" &&
> +	test_expect_success "'worktree add' with '$opts' has mutually exclusive options" '
> +		test_must_fail git worktree add $opts
> +	'
> +}
>
> -test_expect_success '"add" -B/--detach mutually exclusive' '
> -	test_must_fail git worktree add -B poodle --detach bamboo main
> -'
> +test_wt_add_excl -b poodle -B poodle bamboo main
> +test_wt_add_excl -b poodle --orphan poodle bamboo
> +test_wt_add_excl -b poodle --detach bamboo main
> +test_wt_add_excl -B poodle --detach bamboo main
> +test_wt_add_excl -B poodle --detach bamboo main
> +test_wt_add_excl -B poodle --orphan poodle bamboo
> +test_wt_add_excl --orphan poodle --detach bamboo
> +test_wt_add_excl --orphan poodle --no-checkout bamboo
> +test_wt_add_excl --orphan poodle bamboo main

>
> > +static int make_worktree_orphan(const char * ref, const struct add_opts *opts,
> > +				struct strvec *child_env)
> > +{
> > +	int ret;
>
> You can avoid this variable entirely....
>
> > +
> > +	validate_new_branchname(ref, &symref, 0);
> > +	strvec_pushl(&cp.args, "symbolic-ref", "HEAD", symref.buf, NULL);
>
> ...by just calling strbuf_release(&symref); right after this line, we'll
> never need it again, and the strvec will have its own copy.
>
> > +	if (opts->quiet)
> > +		strvec_push(&cp.args, "--quiet");
> > +	strvec_pushv(&cp.env, child_env->v);
>
> So:
>
> > +	ret = run_command(&cp);
> > +	strbuf_release(&symref);
> > +	return ret;
>
> We don't have to carry the "ret" here, and can just do:
>
> 	return run_command(&cp);
>

>
> > +	struct strbuf symref = STRBUF_INIT;
> > +	struct child_process cp = CHILD_PROCESS_INIT;
> > +	cp.git_cmd = 1;
>
> (aside: We usually split up variables & decls, I think this is better
> right before the run_command() line).

> > +}
> > +
> >  static int add_worktree(const char *path, const char *refname,
> >  			const struct add_opts *opts)
> >  {
> > @@ -393,8 +415,9 @@ static int add_worktree(const char *path, const char *refname,
> >  			die_if_checked_out(symref.buf, 0);
> >  	}
> >  	commit = lookup_commit_reference_by_name(refname);
> > -	if (!commit)
> > +	if (!commit && !opts->orphan) {
> >  		die(_("invalid reference: %s"), refname);
> > +	}
>
> We don't add {}'s for one-statement if's like this, see
> CodingGuidelines. So skip the {}'s.
>

> >
> >  	name = worktree_basename(path, &len);
> >  	strbuf_add(&sb, name, path + len - name);
> > @@ -482,10 +505,10 @@ static int add_worktree(const char *path, const char *refname,
> >  	strvec_pushf(&child_env, "%s=%s", GIT_WORK_TREE_ENVIRONMENT, path);
> >  	cp.git_cmd = 1;
> >
> > -	if (!is_branch)
> > +	if (!is_branch && commit) {
> >  		strvec_pushl(&cp.args, "update-ref", "HEAD",
> >  			     oid_to_hex(&commit->object.oid), NULL);
> > -	else {
> > +	} else {
>
> Here that style change is good, even if it inflates the diff size a
> litte bit with the while-at-it fixu-up.
>
> > +	/*
> > +	 * When creating a new branch, new_branch now contains the branch to
> > +	 * create.
> > +	 *
> > +	 * Past this point, new_branch_force can be treated solely as a
> > +	 * boolean flag to indicate whether `-B` was selected.
> > +	 */
> >  	if (new_branch_force) {
> >  		struct strbuf symref = STRBUF_INIT;
> >
>
> I think I commented on this commentary in an earlier round. IMO it could
> just be omitted, as the code is rather self-explanatory.
>
> To the extent that it isn't this commentary just makes things more
> confusing, at least to my reading. It's not explaining what the code is
> doing now, because the very next line after this context (omitted here) is:
>
> 	new_branch = new_branch_force
>
> So we're saying it "can be treated solely as a boolean flag", but it
> isn't being treated as such by the code now.
>
> And the "new_branch now contains the branch to create" is also
> inaccurate, we're about to make it true with that assignment, but (and
> again, I don't think a comment is needed at all) *if* we think that's
> worth commenting on then surely the first paragraph of the comment
> should be split off, and come just before that assignment.

>
> > -	if (new_branch) {
> > +	if (opts.orphan) {
> > +		branch = new_branch;
> > +	} else if (!lookup_commit_reference_by_name(branch)) {
> > +		/*
> > +		 * If `branch` does not reference a valid commit, a new
> > +		 * worktree (and/or branch) cannot be created based off of it.
> > +		 */
>
> I think with the advice added in 3/3 this comment can also just be
> omitted here, as the end result is that the comment will be
> re-explaining something which should be obvious from the inline advice
> string (and if it isn't, that inline string needs improving).

>
> > -test_expect_success '"add" -b/-B mutually exclusive' '
> > -	test_must_fail git worktree add -b poodle -B poodle bamboo main
> > -'
> > -
> > -test_expect_success '"add" -b/--detach mutually exclusive' '
> > -	test_must_fail git worktree add -b poodle --detach bamboo main
> > -'
> > +# Helper function to test mutually exclusive options.
> > +test_wt_add_excl() {
> > +	local opts="$@" &&
> > +	test_expect_success "'worktree add' with '$opts' has mutually exclusive options" '
> > +		test_must_fail git worktree add $opts
> > +	'
> > +}
> >
> > -test_expect_success '"add" -B/--detach mutually exclusive' '
> > -	test_must_fail git worktree add -B poodle --detach bamboo main
> > -'
> > +test_wt_add_excl -b poodle -B poodle bamboo main
> > +test_wt_add_excl -b poodle --orphan poodle bamboo
> > +test_wt_add_excl -b poodle --detach bamboo main
> > +test_wt_add_excl -B poodle --detach bamboo main
> > +test_wt_add_excl -B poodle --detach bamboo main
> > +test_wt_add_excl -B poodle --orphan poodle bamboo
> > +test_wt_add_excl --orphan poodle --detach bamboo
> > +test_wt_add_excl --orphan poodle --no-checkout bamboo
> > +test_wt_add_excl --orphan poodle bamboo main
>
> It's good to see this as a helper function, but I think it would be nice
> to have this split up into its own pre-refactoring commit.
>
> As here we're changing some existing tests that are per-se unrelated,
> just so that they can use this new helper.
>
> This commit could then add tests that use the helper, and which are new
> for --orphan.

> On 22/12/12 09:11AM, Ævar Arnfjörð Bjarmason wrote:
>>
>> > +	struct strbuf symref = STRBUF_INIT;
>> > +	struct child_process cp = CHILD_PROCESS_INIT;
>> > +	cp.git_cmd = 1;
>>
>> (aside: We usually split up variables & decls, I think this is better
>> right before the run_command() line).
>
> Sorry, I'm not quite clear what you mean.

>
> On Mon, Dec 12 2022, Jacob Abel wrote:
>
> > On 22/12/12 09:11AM, Ævar Arnfjörð Bjarmason wrote:
> >>
> >> > +	struct strbuf symref = STRBUF_INIT;
> >> > +	struct child_process cp = CHILD_PROCESS_INIT;
> >> > +	cp.git_cmd = 1;
> >>
> >> (aside: We usually split up variables & decls, I think this is better
> >> right before the run_command() line).
> >
> > Sorry, I'm not quite clear what you mean.
>
> I mean that we usually put two newlines between the last deceleration
> and the start of any code.
>
> And additionally, that the usual pattern for CHILD_PROCESS_INIT is to do
> these "flag" assignments right before the run_command().
>
> See e.g. the code in "check_clean_worktree()"
>
> So following its example would resolve the decl style nit

>  int git_default_advice_config(const char *var, const char *value);
> diff --git a/builtin/worktree.c b/builtin/worktree.c
> index 51b247b2a7..ea306e18de 100644
> --- a/builtin/worktree.c
> +++ b/builtin/worktree.c
> @@ -744,6 +744,14 @@ static int add(int ac, const char **av, const char *prefix)
>  		 * If `branch` does not reference a valid commit, a new
>  		 * worktree (and/or branch) cannot be created based off of it.
>  		 */
> +		advise_if_enabled(ADVICE_WORKTREE_ADD_ORPHAN,
> +			"If you meant to create a worktree containing a new orphan branch\n"
> +			"(branch with no commits) for this repository, e.g. '%s',\n"

> +	test_expect_success "'worktree add' show orphan hint in empty repo w/ $context" '
> +		test_when_finished "rm -rf empty_repo" &&
> +		GIT_DIR="empty_repo" git init --bare &&
> +		test_must_fail git -C empty_repo worktree add $opts 2> actual &&
> +		grep "hint: If you meant to create a worktree containing a new orphan branch" actual
> +	'
> +}
> +
> +test_wt_add_empty_repo_orphan_hint 'DWIM' foobar/
> +test_wt_add_empty_repo_orphan_hint '-b' -b foobar_branch foobar/
> +test_wt_add_empty_repo_orphan_hint '-B' -B foobar_branch foobar/

>
> On Mon, Dec 12 2022, Jacob Abel wrote:
>
> >  int git_default_advice_config(const char *var, const char *value);
> > diff --git a/builtin/worktree.c b/builtin/worktree.c
> > index 51b247b2a7..ea306e18de 100644
> > --- a/builtin/worktree.c
> > +++ b/builtin/worktree.c
> > @@ -744,6 +744,14 @@ static int add(int ac, const char **av, const char *prefix)
> >  		 * If `branch` does not reference a valid commit, a new
> >  		 * worktree (and/or branch) cannot be created based off of it.
> >  		 */
> > +		advise_if_enabled(ADVICE_WORKTREE_ADD_ORPHAN,
> > +			"If you meant to create a worktree containing a new orphan branch\n"
> > +			"(branch with no commits) for this repository, e.g. '%s',\n"
>
> I think this '%s' is just confusing, how is repeating the name of the
> branch at the user (which we're about to mention in the example command)
> helpful?
>
> Shouldn't we just omit it, or reword this to e.g.:
>
> 	If you'd like the '%s' branch to be a worktree containing a
> 	new....
>
>
> > +			"you can do so using the --orphan option:\n"
> > +			"\n"
> > +			"	git worktree add --orphan %s %s\n"
> > +			"\n",

>
> Missing the usual:
>
> 	"Turn this message off by running\n"
> 	"\"git config advice.worktreeAddOrphan false\""
>
> blurb.

>
> > +test_wt_add_empty_repo_orphan_hint() {
> > +	local context="$1" &&
> > +	local opts="${@:2}" &&
>
> This appears to be some shell pseudo-syntax, and my shell hard-errors on
> this.
>
> But as we don't "shift" after the "$1" I don't see how what you
> *probably* meant could work, i.e. we always have arguments, so surely
> we'd always use "$@"?

>
>
> > +	test_expect_success "'worktree add' show orphan hint in empty repo w/ $context" '
> > +		test_when_finished "rm -rf empty_repo" &&
> > +		GIT_DIR="empty_repo" git init --bare &&
> > +		test_must_fail git -C empty_repo worktree add $opts 2> actual &&
> > +		grep "hint: If you meant to create a worktree containing a new orphan branch" actual
> > +	'
> > +}
> > +
> > +test_wt_add_empty_repo_orphan_hint 'DWIM' foobar/
> > +test_wt_add_empty_repo_orphan_hint '-b' -b foobar_branch foobar/
> > +test_wt_add_empty_repo_orphan_hint '-B' -B foobar_branch foobar/
>
> You're just testing how these options interact, so let's have the
> "foobar" part provided by the test function itself.

> On 22/12/12 09:35AM, Ævar Arnfjörð Bjarmason wrote:
>>
>> On Mon, Dec 12 2022, Jacob Abel wrote:
>>
>> >  int git_default_advice_config(const char *var, const char *value);
>> > diff --git a/builtin/worktree.c b/builtin/worktree.c
>> > index 51b247b2a7..ea306e18de 100644
>> > --- a/builtin/worktree.c
>> > +++ b/builtin/worktree.c
>> > @@ -744,6 +744,14 @@ static int add(int ac, const char **av, const char *prefix)
>> >  		 * If `branch` does not reference a valid commit, a new
>> >  		 * worktree (and/or branch) cannot be created based off of it.
>> >  		 */
>> > +		advise_if_enabled(ADVICE_WORKTREE_ADD_ORPHAN,
>> > +			"If you meant to create a worktree containing a new orphan branch\n"
>> > +			"(branch with no commits) for this repository, e.g. '%s',\n"
>>
>> I think this '%s' is just confusing, how is repeating the name of the
>> branch at the user (which we're about to mention in the example command)
>> helpful?
>>
>> Shouldn't we just omit it, or reword this to e.g.:
>>
>> 	If you'd like the '%s' branch to be a worktree containing a
>> 	new....
>>
>>
>> > +			"you can do so using the --orphan option:\n"
>> > +			"\n"
>> > +			"	git worktree add --orphan %s %s\n"
>> > +			"\n",
>
> Done.
>
>>
>> Missing the usual:
>>
>> 	"Turn this message off by running\n"
>> 	"\"git config advice.worktreeAddOrphan false\""
>>
>> blurb.
>
> That blurb is added at runtime by `advise_if_enabled()`. I originally added it
> but it was giving me the line twice so I took it out.

> The following are what I have made for this set of changes (against v4).
>
> diff --git a/builtin/worktree.c b/builtin/worktree.c
> index 8bb1453e0f..38f7a27927 100644
> --- a/builtin/worktree.c
> +++ b/builtin/worktree.c
> @@ -732,12 +732,11 @@ static int add(int ac, const char **av, const char *prefix)
>         } else if (!lookup_commit_reference_by_name(branch)) {
>                 advise_if_enabled(ADVICE_WORKTREE_ADD_ORPHAN,
>                         "If you meant to create a worktree containing a new orphan branch\n"
> -                       "(branch with no commits) for this repository, e.g. '%s',\n"
> -                       "you can do so using the --orphan option:\n"
> +                       "(branch with no commits) for this repository, you can do so\n"
> +                       "using the --orphan option:\n"
>                         "\n"
> -                       "       git worktree add --orphan %s %s\n"
> -                       "\n",
> -                        new_branch, new_branch, path);
> +                       "       git worktree add --orphan %s %s\n",
> +                        new_branch, path);

> -# Helper function to test hints for using --orphan in an empty repo.
>  test_wt_add_empty_repo_orphan_hint() {
> -       local context="$1" &&
> -       local opts="${@:2}" &&
> +       local context="$1"
> +       shift
> +       local opts="$@"
>         test_expect_success "'worktree add' show orphan hint in empty repo w/ $context" '
>                 test_when_finished "rm -rf empty_repo" &&
>                 GIT_DIR="empty_repo" git init --bare &&
> -               test_must_fail git -C empty_repo worktree add $opts 2> actual &&
> +               test_must_fail git -C empty_repo worktree add $opts foobar/ 2> actual &&

>                 grep "hint: If you meant to create a worktree containing a new orphan branch" actual
>         '
>  }
>
> -test_wt_add_empty_repo_orphan_hint 'DWIM' foobar/
> -test_wt_add_empty_repo_orphan_hint '-b' -b foobar_branch foobar/
> -test_wt_add_empty_repo_orphan_hint '-B' -B foobar_branch foobar/
> +test_wt_add_empty_repo_orphan_hint 'DWIM'
> +test_wt_add_empty_repo_orphan_hint '-b' -b foobar_branch
> +test_wt_add_empty_repo_orphan_hint '-B' -B foobar_branch
>
>  test_expect_success 'local clone from linked checkout' '
>         git clone --local here here-clone &&
>
>
> Also, Is there an easier way to debug the `test_expect_success` statements? I
> tried enabling tracing mode but it doesn't seem to trace into those statements.

> On Mon, Dec 12 2022, Jacob Abel wrote:
> > Also, Is there an easier way to debug the `test_expect_success` statements? I
> > tried enabling tracing mode but it doesn't seem to trace into those statements.
>
> Not really, other than just enabling "-x" for the test-lib.sh itself, i.e.:
>
>         sh -x ./t0001-init.sh
>
> I *think* that should work, but I didn't test it...

> On Mon, Dec 12, 2022 at 1:19 PM Ævar Arnfjörð Bjarmason
> <avarab@gmail.com> wrote:
> > On Mon, Dec 12 2022, Jacob Abel wrote:
> > > Also, Is there an easier way to debug the `test_expect_success` statements? I
> > > tried enabling tracing mode but it doesn't seem to trace into those statements.
> >
> > Not really, other than just enabling "-x" for the test-lib.sh itself, i.e.:
> >
> >         sh -x ./t0001-init.sh
> >
> > I *think* that should work, but I didn't test it...
>
> Isn't the question here how to debug the body of a
> test_expect_success? If that's the case, then running the test with -x
> and -i should help:
>
>     ./t001-init.sh -x -i
>
> The -x makes it print all the output as it's executing the body of the
> test_expect_success rather than suppressing it, and -i makes it stop
> as soon as it encounters a failing test, which makes it easier to
> examine the state of that test. After the script aborts (via -i), look
> inside the "trash*" directory. Also, you can insert calls to
> test_pause in the body of a test, which will make it drop into an
> interactive shell session in the trash directory at the point of the
> test_pause, so you can examine the state of the test directly as it
> exists at that point (as opposed to examining it after the test
> failed, as with -i).

> > The following are what I have made for this set of changes (against v4).
> >
> > diff --git a/builtin/worktree.c b/builtin/worktree.c
> > index 8bb1453e0f..38f7a27927 100644
> > --- a/builtin/worktree.c
> > +++ b/builtin/worktree.c
> > @@ -732,12 +732,11 @@ static int add(int ac, const char **av, const char *prefix)
> >         } else if (!lookup_commit_reference_by_name(branch)) {
> >                 advise_if_enabled(ADVICE_WORKTREE_ADD_ORPHAN,
> >                         "If you meant to create a worktree containing a new orphan branch\n"
> > -                       "(branch with no commits) for this repository, e.g. '%s',\n"
> > -                       "you can do so using the --orphan option:\n"
> > +                       "(branch with no commits) for this repository, you can do so\n"
> > +                       "using the --orphan option:\n"
> >                         "\n"
> > -                       "       git worktree add --orphan %s %s\n"
> > -                       "\n",
> > -                        new_branch, new_branch, path);
> > +                       "       git worktree add --orphan %s %s\n",
> > +                        new_branch, path);
>
> Looks good, we'd usually put the "new_branch, path" on the previous line
> (to the extent that it fits within 79 chars.
>
> Also: This string should use _() to mark this for translation.

> diff --git a/builtin/worktree.c b/builtin/worktree.c
> index 4a24d53be1..fccb17f070 100644
> --- a/builtin/worktree.c
> +++ b/builtin/worktree.c
> @@ -17,7 +17,7 @@
>
>  #define BUILTIN_WORKTREE_ADD_USAGE \
>  	N_("git worktree add [-f] [--detach] [--checkout] [--lock [--reason <string>]]\n" \
> -	   "                 [-b <new-branch>] <path> [<commit-ish>]")
> +	   "                 [[-b | -B] <new-branch>] <path> [<commit-ish>]")

> Jacob Abel <jacobabel@nullpo.dev> writes:
>
> > Subject: Re: [PATCH v5 1/4] worktree add: Include -B in usage docs
>
> s/Include/include/;

>
> > While -B behavior is already documented, it was not included in the
> > usage docs for either the man page or the help text.
>
> Good.
>
> > This change fixes
> > that and brings the usage docs in line with how the flags are documented
> > in other commands such as git checkout.
>
> We often just give an order to the codebase to "be like so", before
> describing the effect of following such an order.  I.e.
>
> 	Document "-B" next to where "-b" is already documented, to
> 	bring the usage docs in line with ...

>
> >  'git worktree add' [-f] [--detach] [--checkout] [--lock [--reason <string>]]
> > -		   [-b <new-branch>] <path> [<commit-ish>]
> > +		   [[-b | -B] <new-branch>] <path> [<commit-ish>]
>
> This is wrong, I think.  We want
>
> 		   [(-b | -B) <new-branch>] <path> [<commit-ish>]
>
> instead.
>
> [ a-thing ] means "a-thing can exist here, and it does not have to".
> [ a-thing | another-thing ] means "a-thing or another-thing can
> exist here, but neither has to be here".  [[-b | -B] <new-branch>]
> would allow <new-branch> without either -b or -B, which is not what
> we want.

>
> > diff --git a/builtin/worktree.c b/builtin/worktree.c
> > index 4a24d53be1..fccb17f070 100644
> > --- a/builtin/worktree.c
> > +++ b/builtin/worktree.c
> > @@ -17,7 +17,7 @@
> >
> >  #define BUILTIN_WORKTREE_ADD_USAGE \
> >  	N_("git worktree add [-f] [--detach] [--checkout] [--lock [--reason <string>]]\n" \
> > -	   "                 [-b <new-branch>] <path> [<commit-ish>]")
> > +	   "                 [[-b | -B] <new-branch>] <path> [<commit-ish>]")
>
> Likewise.

> Pull duplicate test code into a function so that additional opt
> combinations can be tested succinctly.
>
> Signed-off-by: Jacob Abel <jacobabel@nullpo.dev>
> ---
>  t/t2400-worktree-add.sh | 20 ++++++++++----------
>  1 file changed, 10 insertions(+), 10 deletions(-)

> +	local opts="$@" &&
> +	test_expect_success "'worktree add' with '$opts' has mutually exclusive options" '
> +		test_must_fail git worktree add $opts
> +	'
> +}

> Jacob Abel <jacobabel@nullpo.dev> writes:
>
> > Pull duplicate test code into a function so that additional opt
> > combinations can be tested succinctly.
> >
> > Signed-off-by: Jacob Abel <jacobabel@nullpo.dev>
> > ---
> >  t/t2400-worktree-add.sh | 20 ++++++++++----------
> >  1 file changed, 10 insertions(+), 10 deletions(-)
>
> OK.
>
> > +# Helper function to test mutually exclusive options.
> > +test_wt_add_excl() {
>
> Have SP on both sides of (), i.e.
>
> 	test_wt_add_excl () {

>
> > +	local opts="$@" &&
> > +	test_expect_success "'worktree add' with '$opts' has mutually exclusive options" '
> > +		test_must_fail git worktree add $opts
> > +	'
> > +}
>
> In this particular case with the given arguments, it does not make a
> difference, but make it a habit to think twice if "$*" is what you
> want when you write "$@".  "$@" does a lot more than just concatenate
> $1, $2, ... with SP in between, and the use of $opts in the message
> merely wants the "concatenate with SP" behaviour, which is what "$*"
> is for.

>
> I actually would write the above if I were doing this patch:
>
>     test_wt_add_excl () {
> 	test_expect_success "'worktree add' with $* has mutually exclusive options" '
> 		test_must_fail git worktree add "$@"
> 	'
>     }
>

> Notice the use of "$@" on the "git worktree add" invocation?  This
> allows callers of test_wt_add_excl pass parameters with SP in it,
> thanks to the magic "$@".  Again, as I said earlier, for these
> callers ...
>
> > +test_wt_add_excl -b poodle -B poodle bamboo main
> > +test_wt_add_excl -b poodle --detach bamboo main
> > +test_wt_add_excl -B poodle --detach bamboo main
>
> ... the distinction does not matter, but
>
>     helper --lock --reason "my reason with multiple words" bamboo main
>
> must be written with "$@", like so:
>
>     helper () {
> 	git worktree add "$@"
>     }
>
> not
>
>     helper () { # BAD
> 	local opt="$@"
> 	git worktree add $opt
>     }
>
> $opt in this case is a SP-concatenated string
>
> 	opt="--lock --reason my reason with multiple words bamboo main"
>
> and passing it without quotes around it to "git worktree add"
> will give only "my" as the parameter to the option "--reason",
> with three extra words on the comand line.

> Adds support for creating an orphan branch when adding a new worktree.
> This functionality is equivalent to git switch's --orphan flag.
>
> The original reason this feature was implemented was to allow a user
> to initialise a new repository using solely the worktree oriented
> workflow.
>
> Current Behavior:
>
> % git init --bare foo.git
> Initialized empty Git repository in /path/to/foo.git/
> % git -C foo.git worktree add main/
> Preparing worktree (new branch 'main')
> fatal: not a valid object name: 'HEAD'
> %
>
> New Behavior:
>
> % git init --bare foo.git
> Initialized empty Git repository in /path/to/foo.git/
> % git -C foo.git worktree add --orphan main main/
> Preparing worktree (new branch 'main')
> %

> Jacob Abel <jacobabel@nullpo.dev> writes:
>
> > Adds support for creating an orphan branch when adding a new worktree.
> > This functionality is equivalent to git switch's --orphan flag.
> >
> > The original reason this feature was implemented was to allow a user
> > to initialise a new repository using solely the worktree oriented
> > workflow.
> >
> > Current Behavior:
> >
> > % git init --bare foo.git
> > Initialized empty Git repository in /path/to/foo.git/
> > % git -C foo.git worktree add main/
> > Preparing worktree (new branch 'main')
> > fatal: not a valid object name: 'HEAD'
> > %
> >
> > New Behavior:
> >
> > % git init --bare foo.git
> > Initialized empty Git repository in /path/to/foo.git/
> > % git -C foo.git worktree add --orphan main main/
> > Preparing worktree (new branch 'main')
> > %
>
> Hmph, I suspect that in reviews for the previous rounds someboddy
> must have brought this up, but I wonder if we can detect the case
> automatically and behave as if "--orphan" were given.  In other
> words, shouldn't the desired outcome (i.e. "worktree add" can be run
> to create an orphan branch even when the original were on an unborn
> branch) become the new behaviour WITHOUT having the user learn new
> option?

>
> If the point of "--orphan" is to create a worktree that checks out a
> yet-to-be-bork branch, whether the original is an empty repository
> or a populated one, then the user may need "--orphan" option, but
> the above illustration is very misleading if that is the intention.
>
> Rather, you should start from a populated repository and explain
> that "worktree add" without "--orphan" (i.e. the current behaviour)
> does not give you an empty tree with empty history, so run an extra
> "switch --orphan" after that.  Then illustrate that you can lose the
> last step with the new option "--orphan".
>
> Or something like that.

>
> > Signed-off-by: Jacob Abel <jacobabel@nullpo.dev>
> > Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
>
> This e-mail coming from you and not from Ævar, with you apparently
> being the author of the patch, makes these two S-o-b lines
> questionable.  What's the chain of custody of the change?  If Ævar
> showed some code changes, and you swallowed that into a larger piece
> of work (i.e. this patch), then the above two lines are swapped.
>

> +test_wt_add_empty_repo_orphan_hint() {
> +	local context="$1"
> +	shift
> +	local opts="$@"
> +	test_expect_success "'worktree add' show orphan hint in empty repo w/ $context" '
> +		test_when_finished "rm -rf empty_repo" &&
> +		GIT_DIR="empty_repo" git init --bare &&
> +		test_must_fail git -C empty_repo worktree add $opts foobar/ 2> actual &&

> Jacob Abel <jacobabel@nullpo.dev> writes:
>
> > Subject: Re: [PATCH v5 4/4] worktree add: Add hint to use --orphan when bad ref
>
> Incomplete sentence that invites "when bad ref, what?"

>
> > +			"	git worktree add --orphan %s %s\n"), new_branch, path
>
> OK.  "git worktree add -b <name-of-branch> <path>" is how you create
> a worktree and have it on a named branch.  And instead of saying -b,
> you would say --orphan.  This sounds like a fairly easy-to-understand
> parallel to how "git checkout [-b/-B/--orphan] name-of-branch" takes
> its parameters.

>
> > +test_wt_add_empty_repo_orphan_hint() {
> > +	local context="$1"
> > +	shift
> > +	local opts="$@"
> > +	test_expect_success "'worktree add' show orphan hint in empty repo w/ $context" '
> > +		test_when_finished "rm -rf empty_repo" &&
> > +		GIT_DIR="empty_repo" git init --bare &&
> > +		test_must_fail git -C empty_repo worktree add $opts foobar/ 2> actual &&
>
> The comments on "$@" (vs "$*") in an earlier step equally applies here.

> Jacob Abel <jacobabel@nullpo.dev> writes:
>
> > +# Saves parameter sequence/array as a string so they can be safely stored in a
> > +# variable and restored with `eval "set -- $arr"`. Sourced from
> > +# https://stackoverflow.com/a/27503158/15064705
>
> Please do not copy from source with unknown licensing terms.

> Yes for these particular tests that should be acceptable. I tried putting an
> alternative together that still provides an avenue for handling quoted
> arguments without the helper [1]. Would this work?
>
> 1. https://lore.kernel.org/git/20221229063823.ij3jjuaar2fsayju@phi/

> Jacob Abel <jacobabel@nullpo.dev> writes:
>
> > Yes for these particular tests that should be acceptable. I tried putting an
> > alternative together that still provides an avenue for handling quoted
> > arguments without the helper [1]. Would this work?
> >
> > 1. https://lore.kernel.org/git/20221229063823.ij3jjuaar2fsayju@phi/
>
> Do we even need to handle an argument with $IFS whitespaces in it in
> these tests?

>
> If not, using "$@" where we pass the args to the commands we run,
> and using "$*" where we merely use it for human readable messages,
> would be sufficient.
>

>
> [...]
>
> So from my understanding of the situation, the only two options that pass all
> the existing tests are either:
>
> A: Use the diff in [1] without the two quote example tests included.
>
> B: Revert the changes to how this was done in v5 [3].
>
> Both of these options work with me however option A will allow test authors to
> easily escape quotes if new tests needed to be added at some point in the future.
>
> 1. https://lore.kernel.org/git/20221229063823.ij3jjuaar2fsayju@phi/
> 2. https://lore.kernel.org/git/xmqqsfhawwqf.fsf@gitster.g/
> 3. https://lore.kernel.org/git/20221220023637.29042-3-jacobabel@nullpo.dev/
> 4. https://lore.kernel.org/git/221228.868risuf5x.gmgdl@evledraar.gmail.com/

> On 22/12/29 03:49PM, Jacob Abel wrote:
>>
>> [...]
>>
>> So from my understanding of the situation, the only two options that pass all
>> the existing tests are either:
>>
>> A: Use the diff in [1] without the two quote example tests included.
>>
>> B: Revert the changes to how this was done in v5 [3].
> ...
> Sorry to poke this but I wanted to confirm which path I should proceed with.
> Both options are functionally complete and it'd just be a matter of choosing
> which version to push out for the revision.

> Jacob Abel <jacobabel@nullpo.dev> writes:
>
> > On 22/12/29 03:49PM, Jacob Abel wrote:
> >>
> >> [...]
> >>
> >> So from my understanding of the situation, the only two options that pass all
> >> the existing tests are either:
> >>
> >> A: Use the diff in [1] without the two quote example tests included.
> >>
> >> B: Revert the changes to how this was done in v5 [3].
> > ...
> > Sorry to poke this but I wanted to confirm which path I should proceed with.
> > Both options are functionally complete and it'd just be a matter of choosing
> > which version to push out for the revision.
>
> I think B. with "$@" -> "$*" (because you only want a flattened
> stringified version of the arguments in $opt to insert into the
> test name string) would be the more sensible avenue.  Let's not
> over-engineer the tests---it is not the point of these new tests to
> ensure that "git worktree add" can take arguments that require to be
> quoted on the command line.
>
> Thanks.

> Current Behavior:
> 
> % git init --bare foo.git
> Initialized empty Git repository in /path/to/foo.git/
> % git -C foo.git worktree add main/
> Preparing worktree (new branch 'main')
> fatal: invalid reference: HEAD
> %
> 
> New Behavior:
> 
> % git init --bare foo.git
> Initialized empty Git repository in /path/to/foo.git/
> % git -C foo.git worktree add main/
> Preparing worktree (new branch 'main')
> hint: If you meant to create a worktree containing a new orphan branch
> hint: (branch with no commits) for this repository, you can do so
> hint: using the --orphan option:
> hint:
> hint:   git worktree add --orphan main ./main
> hint:
> hint: Disable this message with "git config advice.worktreeAddOrphan false"
> fatal: invalid reference: HEAD
> %
> 
> Signed-off-by: Jacob Abel <jacobabel@nullpo.dev>
> ---
>   Documentation/config/advice.txt |  4 ++++
>   advice.c                        |  1 +
>   advice.h                        |  1 +
>   builtin/worktree.c              |  6 ++++++
>   t/t2400-worktree-add.sh         | 17 +++++++++++++++++
>   5 files changed, 29 insertions(+)
> 
> diff --git a/Documentation/config/advice.txt b/Documentation/config/advice.txt
> index a00d0100a8..3e58521613 100644
> --- a/Documentation/config/advice.txt
> +++ b/Documentation/config/advice.txt
> @@ -136,4 +136,8 @@ advice.*::
>   		Advice shown when either linkgit:git-add[1] or linkgit:git-rm[1]
>   		is asked to update index entries outside the current sparse
>   		checkout.
> +	worktreeAddOrphan::
> +		Advice shown when a user tries to create a worktree from an
> +		invalid reference, to instruct how to create a new orphan
> +		branch instead.
>   --
> diff --git a/advice.c b/advice.c
> index fd18968943..53e91fdb85 100644
> --- a/advice.c
> +++ b/advice.c
> @@ -75,6 +75,7 @@ static struct {
>   	[ADVICE_SUBMODULES_NOT_UPDATED] 		= { "submodulesNotUpdated", 1 },
>   	[ADVICE_UPDATE_SPARSE_PATH]			= { "updateSparsePath", 1 },
>   	[ADVICE_WAITING_FOR_EDITOR]			= { "waitingForEditor", 1 },
> +	[ADVICE_WORKTREE_ADD_ORPHAN]			= { "worktreeAddOrphan", 1 },
>   };
> 
>   static const char turn_off_instructions[] =
> diff --git a/advice.h b/advice.h
> index 07e0f76833..919d8c0064 100644
> --- a/advice.h
> +++ b/advice.h
> @@ -50,6 +50,7 @@ struct string_list;
>   	ADVICE_UPDATE_SPARSE_PATH,
>   	ADVICE_WAITING_FOR_EDITOR,
>   	ADVICE_SKIPPED_CHERRY_PICKS,
> +	ADVICE_WORKTREE_ADD_ORPHAN,
>   };
> 
>   int git_default_advice_config(const char *var, const char *value);
> diff --git a/builtin/worktree.c b/builtin/worktree.c
> index ac82c5feda..d975628353 100644
> --- a/builtin/worktree.c
> +++ b/builtin/worktree.c
> @@ -730,6 +730,12 @@ static int add(int ac, const char **av, const char *prefix)
>   	if (opts.orphan) {
>   		branch = new_branch;
>   	} else if (!lookup_commit_reference_by_name(branch)) {
> +		advise_if_enabled(ADVICE_WORKTREE_ADD_ORPHAN,
> +			_("If you meant to create a worktree containing a new orphan branch\n"
> +			"(branch with no commits) for this repository, you can do so\n"
> +			"using the --orphan option:\n"
> +			"\n"
> +			"	git worktree add --orphan %s %s\n"), new_branch, path);
>   		die(_("invalid reference: %s"), branch);
>   	} else if (new_branch) {
>   		struct child_process cp = CHILD_PROCESS_INIT;
> diff --git a/t/t2400-worktree-add.sh b/t/t2400-worktree-add.sh
> index 43c95e6426..f43de59117 100755
> --- a/t/t2400-worktree-add.sh
> +++ b/t/t2400-worktree-add.sh
> @@ -397,6 +397,23 @@ test_expect_success '"add" worktree with orphan branch, lock, and reason' '
>   	test_cmp expect .git/worktrees/orphan-with-lock-reason/locked
>   '
> 
> +test_wt_add_empty_repo_orphan_hint () {
> +	local context="$1"
> +	shift
> +	local arr=$(save_param_arr "$@")
> +	test_expect_success "'worktree add' show orphan hint in empty repo w/ $context" '
> +		eval "set -- $arr" &&
> +		test_when_finished "rm -rf empty_repo" &&
> +		GIT_DIR="empty_repo" git init --bare &&
> +		test_must_fail git -C empty_repo worktree add "$@" foobar/ 2> actual &&
> +		grep "hint: If you meant to create a worktree containing a new orphan branch" actual
> +	'
> +}
> +
> +test_wt_add_empty_repo_orphan_hint 'DWIM'
> +test_wt_add_empty_repo_orphan_hint '-b' -b foobar_branch
> +test_wt_add_empty_repo_orphan_hint '-B' -B foobar_branch
> +
>   test_expect_success 'local clone from linked checkout' '
>   	git clone --local here here-clone &&
>   	( cd here-clone && git fsck )
> --
> 2.38.2
> 
> 

>   * Added save_param_arr() to preserve "$@" from being reset by
>     test_expect_success() in test_wt_add_excl() (2/4).
> [...]
> 2:  3d8b26f9d6 ! 2:  c03c112f79 worktree add: refactor opt exclusion tests
>     @@ t/t2400-worktree-add.sh: test_expect_success '"add" no auto-vivify with --detach
>      -test_expect_success '"add" -b/-B mutually exclusive' '
>      -	test_must_fail git worktree add -b poodle -B poodle bamboo main
>      -'
>     --
>     ++# Saves parameter sequence/array as a string so they can be safely stored in a
>     ++# variable and restored with `eval "set -- $arr"`. Sourced from
>     ++# https://stackoverflow.com/a/27503158/15064705
>     ++save_param_arr () {
>     ++	local i
>     ++	for i;
>     ++	do
>     ++		# For each argument:
>     ++		# 1. Append "\n" after each entry
>     ++		# 2. Convert "'" into "'\''"
>     ++		# 3. Prepend "'" before each entry
>     ++		# 4. Append " \" after each entry
>     ++		printf "%s\\n" "$i" | sed "
>     ++			s/'/'\\\\''/g
>     ++			1s/^/'/
>     ++			\$s/\$/' \\\\/
>     ++		"
>     ++	done
>     ++	echo " "
>     ++}
>     +
> [...]
>       ## Documentation/config/advice.txt ##
>     @@ t/t2400-worktree-add.sh: test_expect_success '"add" worktree with orphan branch,
>       	test_cmp expect .git/worktrees/orphan-with-lock-reason/locked
>       '
>
>     -+test_wt_add_empty_repo_orphan_hint() {
>     ++test_wt_add_empty_repo_orphan_hint () {
>      +	local context="$1"
>      +	shift
>     -+	local opts="$@"
>     ++	local arr=$(save_param_arr "$@")
>      +	test_expect_success "'worktree add' show orphan hint in empty repo w/ $context" '
>     ++		eval "set -- $arr" &&
>      +		test_when_finished "rm -rf empty_repo" &&
>      +		GIT_DIR="empty_repo" git init --bare &&
>     -+		test_must_fail git -C empty_repo worktree add $opts foobar/ 2> actual &&
>     ++		test_must_fail git -C empty_repo worktree add "$@" foobar/ 2> actual &&
>      +		grep "hint: If you meant to create a worktree containing a new orphan branch" actual
>      +	'
>      +}

>
> On Wed, Dec 28 2022, Jacob Abel wrote:
>
> >   * Added save_param_arr() to preserve "$@" from being reset by
> >     test_expect_success() in test_wt_add_excl() (2/4).
> > [...]
> > 2:  3d8b26f9d6 ! 2:  c03c112f79 worktree add: refactor opt exclusion tests
> >     @@ t/t2400-worktree-add.sh: test_expect_success '"add" no auto-vivify with --detach
> >      -test_expect_success '"add" -b/-B mutually exclusive' '
> >      -	test_must_fail git worktree add -b poodle -B poodle bamboo main
> >      -'
> >     --
> >     ++# Saves parameter sequence/array as a string so they can be safely stored in a
> >     ++# variable and restored with `eval "set -- $arr"`. Sourced from
> >     ++# https://stackoverflow.com/a/27503158/15064705
> >     ++save_param_arr () {
> >     ++	local i
> >     ++	for i;
> >     ++	do
> >     ++		# For each argument:
> >     ++		# 1. Append "\n" after each entry
> >     ++		# 2. Convert "'" into "'\''"
> >     ++		# 3. Prepend "'" before each entry
> >     ++		# 4. Append " \" after each entry
> >     ++		printf "%s\\n" "$i" | sed "
> >     ++			s/'/'\\\\''/g
> >     ++			1s/^/'/
> >     ++			\$s/\$/' \\\\/
> >     ++		"
> >     ++	done
> >     ++	echo " "
> >     ++}
> >     +
> > [...]
> >       ## Documentation/config/advice.txt ##
> >     @@ t/t2400-worktree-add.sh: test_expect_success '"add" worktree with orphan branch,
> >       	test_cmp expect .git/worktrees/orphan-with-lock-reason/locked
> >       '
> >
> >     -+test_wt_add_empty_repo_orphan_hint() {
> >     ++test_wt_add_empty_repo_orphan_hint () {
> >      +	local context="$1"
> >      +	shift
> >     -+	local opts="$@"
> >     ++	local arr=$(save_param_arr "$@")
> >      +	test_expect_success "'worktree add' show orphan hint in empty repo w/ $context" '
> >     ++		eval "set -- $arr" &&
> >      +		test_when_finished "rm -rf empty_repo" &&
> >      +		GIT_DIR="empty_repo" git init --bare &&
> >     -+		test_must_fail git -C empty_repo worktree add $opts foobar/ 2> actual &&
> >     ++		test_must_fail git -C empty_repo worktree add "$@" foobar/ 2> actual &&
> >      +		grep "hint: If you meant to create a worktree containing a new orphan branch" actual
> >      +	'
> >      +}
>
> The rest of this looks good to me, but this bit looks like you went down
> the rabbit hole of responding to Junio's feedback in
> https://lore.kernel.org/git/xmqqsfhawwqf.fsf@gitster.g/
>
> I think as we're not dealing with any quoted arguments here it's not
> worth copy/pasting some code to do shell quoting from StackOverflow,
> i.e. for this series this squashed at the tip passes all the tests:

>
> 	diff --git a/t/t2400-worktree-add.sh b/t/t2400-worktree-add.sh
> 	index f43de591173..e5b01583cf2 100755
> 	--- a/t/t2400-worktree-add.sh
> 	+++ b/t/t2400-worktree-add.sh
> 	@@ -298,33 +298,11 @@ test_expect_success '"add" no auto-vivify with --detach and <branch> omitted' '
> 	 	test_must_fail git -C mish/mash symbolic-ref HEAD
> 	 '
>
> 	-# Saves parameter sequence/array as a string so they can be safely stored in a
> 	-# variable and restored with `eval "set -- $arr"`. Sourced from
> 	-# https://stackoverflow.com/a/27503158/15064705
> 	-save_param_arr () {
> 	-	local i
> 	-	for i;
> 	-	do
> 	-		# For each argument:
> 	-		# 1. Append "\n" after each entry
> 	-		# 2. Convert "'" into "'\''"
> 	-		# 3. Prepend "'" before each entry
> 	-		# 4. Append " \" after each entry
> 	-		printf "%s\\n" "$i" | sed "
> 	-			s/'/'\\\\''/g
> 	-			1s/^/'/
> 	-			\$s/\$/' \\\\/
> 	-		"
> 	-	done
> 	-	echo " "
> 	-}
> 	-
> 	 # Helper function to test mutually exclusive options.
> 	 test_wt_add_excl () {
> 	-	local arr=$(save_param_arr "$@")
> 	-	test_expect_success "'worktree add' with $* has mutually exclusive options" '
> 	-		eval "set -- $arr" &&
> 	-		test_must_fail git worktree add "$@"
> 	+	local args="$@" &&
> 	+	test_expect_success "'worktree add' with '$args' has mutually exclusive options" '
> 	+		test_must_fail git worktree add $args
> 	 	'
> 	 }
>
> 	@@ -398,21 +376,20 @@ test_expect_success '"add" worktree with orphan branch, lock, and reason' '
> 	 '
>
> 	 test_wt_add_empty_repo_orphan_hint () {
> 	-	local context="$1"
> 	-	shift
> 	-	local arr=$(save_param_arr "$@")
> 	+	local context="$1" &&
> 	+	shift &&
> 	+	local args="$@" &&
> 	 	test_expect_success "'worktree add' show orphan hint in empty repo w/ $context" '
> 	-		eval "set -- $arr" &&
> 	 		test_when_finished "rm -rf empty_repo" &&
> 	 		GIT_DIR="empty_repo" git init --bare &&
> 	-		test_must_fail git -C empty_repo worktree add "$@" foobar/ 2> actual &&
> 	+		test_must_fail git -C empty_repo worktree add $args foobar/ 2> actual &&
> 	 		grep "hint: If you meant to create a worktree containing a new orphan branch" actual
> 	 	'
> 	 }
>
> 	-test_wt_add_empty_repo_orphan_hint 'DWIM'
> 	-test_wt_add_empty_repo_orphan_hint '-b' -b foobar_branch
> 	-test_wt_add_empty_repo_orphan_hint '-B' -B foobar_branch
> 	+test_wt_add_empty_repo_orphan_hint DWIM
> 	+test_wt_add_empty_repo_orphan_hint -b -b foobar_branch
> 	+test_wt_add_empty_repo_orphan_hint -B -B foobar_branch
>
> 	 test_expect_success 'local clone from linked checkout' '
> 	 	git clone --local here here-clone &&
>
> Note also that you lost the &&-chaining.

>
> If we do want to be slightly paranoid about it, doesn't just creating a:
>
> 	local args_str="$*" &&
>
> And then using that in the description argument to test_expect_success()
> also address Junio's feedback, without the need for this quoting helper?

> On 22/12/28 09:01AM, Ævar Arnfjörð Bjarmason wrote:
>>
>> On Wed, Dec 28 2022, Jacob Abel wrote:
>> [...]
>> The rest of this looks good to me, but this bit looks like you went down
>> the rabbit hole of responding to Junio's feedback in
>> https://lore.kernel.org/git/xmqqsfhawwqf.fsf@gitster.g/
>>
>> I think as we're not dealing with any quoted arguments here it's not
>> worth copy/pasting some code to do shell quoting from StackOverflow,
>> i.e. for this series this squashed at the tip passes all the tests:
>
> Understood.
> [...]
>>
>> If we do want to be slightly paranoid about it, doesn't just creating a:
>>
>> 	local args_str="$*" &&
>>
>> And then using that in the description argument to test_expect_success()
>> also address Junio's feedback, without the need for this quoting helper?
>
> Below is what I have come up with while still not needing the
> quoting helper. Could this work as an alternative?
>
> It doesn't handle quotes properly without a bit of help from the
> test author but it can handle them as long as you double escape the string.
>
> The diff also includes slight tweaks to the tests themselves to better verify
> the behavior.
>
> Note: The two extra tests added in the diff wouldn't be in the next revision but they
> are there to demonstrate that things work as expected with this change.
>
> [...]
>  # Helper function to test mutually exclusive options.
> +#
> +# Note: Any arguments that contain spaces must be double and single quoted, ex:
> +# test_wt_add_excl -b poodle --detach bamboo --lock --reason "'the reason'" main
>  test_wt_add_excl () {
> -	local arr=$(save_param_arr "$@")
> -	test_expect_success "'worktree add' with $* has mutually exclusive options" '
> -		eval "set -- $arr" &&
> -		test_must_fail git worktree add "$@"
> -	'
> +	test_expect_success "'worktree add' with $* has mutually exclusive options" "
> +		test_must_fail git worktree add $* 2>actual &&
> +		grep -P 'fatal:( options)? .* cannot be used together' actual
> +	"
>  }
>
> +test_wt_add_excl -b poodle --detach bamboo --lock --reason "'the reason'" main
>  test_wt_add_excl -b poodle -B poodle bamboo main
>  test_wt_add_excl -b poodle --detach bamboo main
>  test_wt_add_excl -B poodle --detach bamboo main
> @@ -397,19 +379,22 @@ test_expect_success '"add" worktree with orphan branch, lock, and reason' '
>  	test_cmp expect .git/worktrees/orphan-with-lock-reason/locked
>  '
>
> +# Note: Any arguments (except the first argument) that contain spaces must be
> +# double and single quoted, ex:
> +# test_wt_add_empty_repo_orphan_hint 'the context' --lock --reason "'the reason'"
> [...]
> +test_wt_add_empty_repo_orphan_hint 'the context' --lock --reason "'the reason'"
>  test_wt_add_empty_repo_orphan_hint 'DWIM'
>  test_wt_add_empty_repo_orphan_hint '-b' -b foobar_branch
>  test_wt_add_empty_repo_orphan_hint '-B' -B foobar_branch

>
> On Thu, Dec 29 2022, Jacob Abel wrote:
> >
> > [...]
> > Note: The two extra tests added in the diff wouldn't be in the next revision but they
> > are there to demonstrate that things work as expected with this change.
> >
> > [...]
> > +test_wt_add_excl -b poodle --detach bamboo --lock --reason "'the reason'" main
> > [...]
> > +test_wt_add_empty_repo_orphan_hint 'the context' --lock --reason "'the reason'"
>
> I haven't even tested this, but I'm still confused here, isn't this just
> a solution in seach of a problem?
>
> To answer your question above: Yes, you can come up with shellscript
> code that handles this sort of quoting problem, but it's generally a Bad
> Idea and should be avoided.
>
> *If* a function needs to take arguments that are quoted it's much better
> to "unpack" those arguments in full, and then pass them on, see e.g. how
> the "test_commit" helper in test-lib-functions.sh does it.
>
> But in this case there was no need whatsoever for doing any of this, as
> none of the tests wanted to pass such arguments, they didn't need to be
> quoted at all.
>
> But now for this reply you've come up with one such test, hence the
> "solution in search of a problem?" above.
>
> I.e. is this a useful test, or just an excercise to stress generic quote
> handling we don't really need?

>
> I originally suggested creating these trivial helpers in an earlier
> round because it avoided the copy/pasting of a series of tests, I think
> the v5 you had
> (https://lore.kernel.org/git/20221212014003.20290-3-jacobabel@nullpo.dev/)
> struck the right balance there, although as Junio noted it might need
> the tweaking for $@ v.s. $*.
>
> But once we have to handle quoted arguments the better solution is to
> just ... not use that helper. I.e. there's no reason you can't just do:
>
> 	test_wt_add_excl -b poodle -B poodle bamboo main
> 	test_wt_add_excl -b poodle --orphan poodle bamboo
> 	[...]
>
> Then:
>
> 	test_expect_success 'worktree add with quoted --reason arguments and --orphan' '
> 		test_must_fail git worktree add  --orphan poodle --detach bamboo --reason "'\''blah blah'\''"
> 	'
>
> You don't need to make test_wt_add_excl() handle that case.

> +test_wt_add_excl () {
> +	local opts="$*" &&
> +	test_expect_success "'worktree add' with '$opts' has mutually exclusive options" '
> +		test_must_fail git worktree add $opts 2>actual &&
> +		grep -P "fatal:( options)? .* cannot be used together" actual
> +	'
> +}

> Jacob Abel <jacobabel@nullpo.dev> writes:
>
> > +test_wt_add_excl () {
> > +	local opts="$*" &&
> > +	test_expect_success "'worktree add' with '$opts' has mutually exclusive options" '
> > +		test_must_fail git worktree add $opts 2>actual &&
> > +		grep -P "fatal:( options)? .* cannot be used together" actual
> > +	'
> > +}
>
> Of course, "grep -P" is non-portable and CI jobs are easily broken.
> Isn't -E (ERE) sufficient here?
>

> While working with the worktree based git workflow, I realised that setting
> up a new git repository required switching between the traditional and
> worktree based workflows. Searching online I found a SO answer [1] which
> seemed to support this and which indicated that adding support for this should
> not be technically difficult.
>
> This patchset has four parts:
>   * adding `-B` to the usage docs (noticed during dev and it seemed too small
>     to justify a separate submission)
>   * adding a helper fn to simplify testing for mutual exclusion of options
>     in `t/t2400-worktree-add.sh`
>   * adding orphan branch functionality (as is present in `git-switch`)
>     to `git-worktree-add`
>   * adding an advise for using --orphan when `git worktree add` fails due to
>     a bad ref.
>
> Changes from v6:
>   * Removed helper save_param_arr() introduced in v6 from t2400 (2/4) [2].
>   * Reverted changes introduced in v6 to test_wt_add_excl() from t2400 (2/4) [3].
>   * Changed test_wt_add_excl() to use `local opts="$*"` (2/4) [3].
>   * Added check to test_wt_add_excl() to better validate test results (2/4).
>   * Re-add &&-chains to test_wt_add_excl() (2/4) [4].
>   * Reverted changes introduced in v6 to test_wt_add_empty_repo_orphan_hint()
>     from t2400 (4/4) [3].
>   * Changed test_wt_add_empty_repo_orphan_hint() to use `local opts="$*"` (4/4) [3].
>   * Added check to test_wt_add_empty_repo_orphan_hint() to better validate test
>     results (4/4).
>   * Re-add &&-chains to test_wt_add_empty_repo_orphan_hint() (2/4) [4].

> Range-diff against v6:
> 1:  a9ef3eca7b = 1:  a9ef3eca7b worktree add: include -B in usage docs
> 2:  c03c112f79 ! 2:  d124cc481c worktree add: refactor opt exclusion tests
>     @@ t/t2400-worktree-add.sh: test_expect_success '"add" no auto-vivify with --detach
>      -test_expect_success '"add" -b/-B mutually exclusive' '
>      -	test_must_fail git worktree add -b poodle -B poodle bamboo main
>      -'
>     -+# Saves parameter sequence/array as a string so they can be safely stored in a
>     -+# variable and restored with `eval "set -- $arr"`. Sourced from
>     -+# https://stackoverflow.com/a/27503158/15064705
>     -+save_param_arr () {
>     -+	local i
>     -+	for i;
>     -+	do
>     -+		# For each argument:
>     -+		# 1. Append "\n" after each entry
>     -+		# 2. Convert "'" into "'\''"
>     -+		# 3. Prepend "'" before each entry
>     -+		# 4. Append " \" after each entry
>     -+		printf "%s\\n" "$i" | sed "
>     -+			s/'/'\\\\''/g
>     -+			1s/^/'/
>     -+			\$s/\$/' \\\\/
>     -+		"
>     -+	done
>     -+	echo " "
>     -+}
>     -
>     +-
>      -test_expect_success '"add" -b/--detach mutually exclusive' '
>      -	test_must_fail git worktree add -b poodle --detach bamboo main
>      -'

>      +# Helper function to test mutually exclusive options.
>     ++#
>     ++# Note: Quoted arguments containing spaces are not supported.
>      +test_wt_add_excl () {
>     -+	local arr=$(save_param_arr "$@")
>     -+	test_expect_success "'worktree add' with $* has mutually exclusive options" '
>     -+		eval "set -- $arr" &&
>     -+		test_must_fail git worktree add "$@"
>     ++	local opts="$*" &&
>     ++	test_expect_success "'worktree add' with '$opts' has mutually exclusive options" '
>     ++		test_must_fail git worktree add $opts 2>actual &&
>     ++		grep -P "fatal:( options)? .* cannot be used together" actual

>
> On Sat, Jan 07 2023, Jacob Abel wrote:
>
> > [...]
>
> This round looks good to me, except for a small tiny (and new)
> portability issue that needs fixing:
>
> > [...]
>
> Good to get rid of this.
>
> > [...]
> >     ++		grep -P "fatal:( options)? .* cannot be used together" actual
>
> This is the new unportable code, the "-P" option is specific to GNU
> grep, and here you're relying on the "?" syntax along with other
> ERE-like syntax.
>
> It looks like the minimal fix here is to just change -P to -E, which we
> can use, you're not using any PCRE-syntax, but just syntax that's part
> of ERE.

> On 23/01/09 01:26PM, Ævar Arnfjörð Bjarmason wrote:
>>
>> On Sat, Jan 07 2023, Jacob Abel wrote:
>>
>> > [...]
>>
>> This round looks good to me, except for a small tiny (and new)
>> portability issue that needs fixing:
>>
>> > [...]
>>
>> Good to get rid of this.
>>
>> > [...]
>> >     ++		grep -P "fatal:( options)? .* cannot be used together" actual
>>
>> This is the new unportable code, the "-P" option is specific to GNU
>> grep, and here you're relying on the "?" syntax along with other
>> ERE-like syntax.
>>
>> It looks like the minimal fix here is to just change -P to -E, which we
>> can use, you're not using any PCRE-syntax, but just syntax that's part
>> of ERE.
>
> Understood. I have made the change and prepared a new revision already if this
> is all that needs to be changed. I can submit it at any time however I was
> unsure of whether it was considered bad etiquette to submit new revisions so
> close to each other.

>
> On Mon, Jan 09 2023, Jacob Abel wrote:
>
> > On 23/01/09 01:26PM, Ævar Arnfjörð Bjarmason wrote:
> >>
> >> On Sat, Jan 07 2023, Jacob Abel wrote:
> >>
> >> > [...]
> >>
> >> This round looks good to me, except for a small tiny (and new)
> >> portability issue that needs fixing:
> >>
> >> > [...]
> >>
> >> Good to get rid of this.
> >>
> >> > [...]
> >> >     ++		grep -P "fatal:( options)? .* cannot be used together" actual
> >>
> >> This is the new unportable code, the "-P" option is specific to GNU
> >> grep, and here you're relying on the "?" syntax along with other
> >> ERE-like syntax.
> >>
> >> It looks like the minimal fix here is to just change -P to -E, which we
> >> can use, you're not using any PCRE-syntax, but just syntax that's part
> >> of ERE.
> >
> > Understood. I have made the change and prepared a new revision already if this
> > is all that needs to be changed. I can submit it at any time however I was
> > unsure of whether it was considered bad etiquette to submit new revisions so
> > close to each other.
>
> It would be good to have that new revision soon in this case.
>
> Generally, if you submit a series you're expected to wait to give more
> comments time to trickle in.
>
> But as a series gets more "ready" a re-roll can come sooner, e.g. in
> this case where at least I'm fairly confident that this is (knock on
> wood) "the last bug".
>
> So once that version's in Junio can hopefully queue it for graduation to
> next soon-ish.

> Adds support for creating an orphan branch when adding a new worktree.
> This functionality is equivalent to git switch's --orphan flag.
> 
> The original reason this feature was implemented was to allow a user
> to initialise a new repository using solely the worktree oriented
> workflow.
> 
> Current Behavior:
> % git -C foo.git --no-pager branch -l
> + main
> % git -C foo.git worktree add main/
> Preparing worktree (new branch 'main')
> HEAD is now at 6c93a75 a commit
> %
> 
> % git init bar.git
> Initialized empty Git repository in /path/to/bar.git/
> % git -C bar.git --no-pager branch -l
> 
> % git -C bar.git worktree add main/
> Preparing worktree (new branch 'main')
> fatal: not a valid object name: 'HEAD'
> %
> 
> New Behavior:
> 
> % git -C foo.git --no-pager branch -l
> + main
> % git -C foo.git worktree add main/
> Preparing worktree (new branch 'main')
> HEAD is now at 6c93a75 a commit
> %
> 
> % git init --bare bar.git
> Initialized empty Git repository in /path/to/bar.git/
> % git -C bar.git --no-pager branch -l
> 
> % git -C bar.git worktree add main/
> Preparing worktree (new branch 'main')
> fatal: invalid reference: HEAD
> % git -C bar.git worktree add --orphan main main/

> Preparing worktree (new branch 'main')
> %
> 
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> Signed-off-by: Jacob Abel <jacobabel@nullpo.dev>
> ---
>   Documentation/git-worktree.txt | 15 +++++++++
>   builtin/worktree.c             | 59 ++++++++++++++++++++++++++++++----
>   t/t2400-worktree-add.sh        | 53 ++++++++++++++++++++++++++++++
>   3 files changed, 120 insertions(+), 7 deletions(-)
> 
> diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt
> index b9c12779f1..d78460c29c 100644
> --- a/Documentation/git-worktree.txt
> +++ b/Documentation/git-worktree.txt
> @@ -11,6 +11,8 @@ SYNOPSIS
>   [verse]
>   'git worktree add' [-f] [--detach] [--checkout] [--lock [--reason <string>]]
>   		   [(-b | -B) <new-branch>] <path> [<commit-ish>]
> +'git worktree add' [-f] [--lock [--reason <string>]]
> +		   --orphan <new-branch> <path>
>   'git worktree list' [-v | --porcelain [-z]]
>   'git worktree lock' [--reason <string>] <worktree>
>   'git worktree move' <worktree> <new-path>
> @@ -95,6 +97,15 @@ exist, a new branch based on `HEAD` is automatically created as if
>   `-b <branch>` was given.  If `<branch>` does exist, it will be checked out
>   in the new worktree, if it's not checked out anywhere else, otherwise the
>   command will refuse to create the worktree (unless `--force` is used).
> ++
> +------------
> +$ git worktree add --orphan <branch> <path>
> +------------
> ++
> +Create a worktree containing no files, with an empty index, and associated
> +with a new orphan branch named `<branch>`. The first commit made on this new
> +branch will have no parents and will be the root of a new history disconnected
> +from any other branches.
> 
>   list::
> 
> @@ -222,6 +233,10 @@ This can also be set up as the default behaviour by using the
>   	With `prune`, do not remove anything; just report what it would
>   	remove.
> 
> +--orphan <new-branch>::
> +	With `add`, make the new worktree and index empty, associating
> +	the worktree with a new orphan branch named `<new-branch>`.
> +
>   --porcelain::
>   	With `list`, output in an easy-to-parse format for scripts.
>   	This format will remain stable across Git versions and regardless of user
> diff --git a/builtin/worktree.c b/builtin/worktree.c
> index ddb33f48a0..ac82c5feda 100644
> --- a/builtin/worktree.c
> +++ b/builtin/worktree.c
> @@ -17,7 +17,10 @@
> 
>   #define BUILTIN_WORKTREE_ADD_USAGE \
>   	N_("git worktree add [-f] [--detach] [--checkout] [--lock [--reason <string>]]\n" \
> -	   "                 [(-b | -B) <new-branch>] <path> [<commit-ish>]")
> +	   "                 [(-b | -B) <new-branch>] <path> [<commit-ish>]"), \
> +	N_("git worktree add [-f] [--lock [--reason <string>]]\n" \
> +	   "                 --orphan <new-branch> <path>")
> +
>   #define BUILTIN_WORKTREE_LIST_USAGE \
>   	N_("git worktree list [-v | --porcelain [-z]]")
>   #define BUILTIN_WORKTREE_LOCK_USAGE \
> @@ -90,6 +93,7 @@ struct add_opts {
>   	int detach;
>   	int quiet;
>   	int checkout;
> +	int orphan;
>   	const char *keep_locked;
>   };
> 
> @@ -364,6 +368,22 @@ static int checkout_worktree(const struct add_opts *opts,
>   	return run_command(&cp);
>   }
> 
> +static int make_worktree_orphan(const char * ref, const struct add_opts *opts,
> +				struct strvec *child_env)
> +{
> +	struct strbuf symref = STRBUF_INIT;
> +	struct child_process cp = CHILD_PROCESS_INIT;
> +
> +	validate_new_branchname(ref, &symref, 0);
> +	strvec_pushl(&cp.args, "symbolic-ref", "HEAD", symref.buf, NULL);
> +	if (opts->quiet)
> +		strvec_push(&cp.args, "--quiet");
> +	strvec_pushv(&cp.env, child_env->v);
> +	strbuf_release(&symref);
> +	cp.git_cmd = 1;
> +	return run_command(&cp);
> +}
> +
>   static int add_worktree(const char *path, const char *refname,
>   			const struct add_opts *opts)
>   {
> @@ -393,7 +413,7 @@ static int add_worktree(const char *path, const char *refname,
>   			die_if_checked_out(symref.buf, 0);
>   	}
>   	commit = lookup_commit_reference_by_name(refname);
> -	if (!commit)
> +	if (!commit && !opts->orphan)
>   		die(_("invalid reference: %s"), refname);
> 
>   	name = worktree_basename(path, &len);
> @@ -482,10 +502,10 @@ static int add_worktree(const char *path, const char *refname,
>   	strvec_pushf(&child_env, "%s=%s", GIT_WORK_TREE_ENVIRONMENT, path);
>   	cp.git_cmd = 1;
> 
> -	if (!is_branch)
> +	if (!is_branch && commit) {
>   		strvec_pushl(&cp.args, "update-ref", "HEAD",
>   			     oid_to_hex(&commit->object.oid), NULL);
> -	else {
> +	} else {
>   		strvec_pushl(&cp.args, "symbolic-ref", "HEAD",
>   			     symref.buf, NULL);
>   		if (opts->quiet)
> @@ -497,6 +517,10 @@ static int add_worktree(const char *path, const char *refname,
>   	if (ret)
>   		goto done;
> 
> +	if (opts->orphan &&
> +	    (ret = make_worktree_orphan(refname, opts, &child_env)))
> +		goto done;
> +
>   	if (opts->checkout &&
>   	    (ret = checkout_worktree(opts, &child_env)))
>   		goto done;
> @@ -516,7 +540,7 @@ static int add_worktree(const char *path, const char *refname,
>   	 * Hook failure does not warrant worktree deletion, so run hook after
>   	 * is_junk is cleared, but do return appropriate code when hook fails.
>   	 */
> -	if (!ret && opts->checkout) {
> +	if (!ret && opts->checkout && !opts->orphan) {
>   		struct run_hooks_opt opt = RUN_HOOKS_OPT_INIT;
> 
>   		strvec_pushl(&opt.env, "GIT_DIR", "GIT_WORK_TREE", NULL);
> @@ -605,6 +629,7 @@ static int add(int ac, const char **av, const char *prefix)
>   	char *path;
>   	const char *branch;
>   	const char *new_branch = NULL;
> +	const char *orphan_branch = NULL;
>   	const char *opt_track = NULL;
>   	const char *lock_reason = NULL;
>   	int keep_locked = 0;
> @@ -616,6 +641,8 @@ static int add(int ac, const char **av, const char *prefix)
>   			   N_("create a new branch")),
>   		OPT_STRING('B', NULL, &new_branch_force, N_("branch"),
>   			   N_("create or reset a branch")),
> +		OPT_STRING(0, "orphan", &orphan_branch, N_("branch"),
> +			   N_("new unparented branch")),
>   		OPT_BOOL('d', "detach", &opts.detach, N_("detach HEAD at named commit")),
>   		OPT_BOOL(0, "checkout", &opts.checkout, N_("populate the new working tree")),
>   		OPT_BOOL(0, "lock", &keep_locked, N_("keep the new working tree locked")),
> @@ -633,8 +660,20 @@ static int add(int ac, const char **av, const char *prefix)
>   	memset(&opts, 0, sizeof(opts));
>   	opts.checkout = 1;
>   	ac = parse_options(ac, av, prefix, options, git_worktree_add_usage, 0);
> +	opts.orphan = !!orphan_branch;
>   	if (!!opts.detach + !!new_branch + !!new_branch_force > 1)
>   		die(_("options '%s', '%s', and '%s' cannot be used together"), "-b", "-B", "--detach");
> +	if (!!opts.detach + !!opts.orphan + !!new_branch + !!new_branch_force > 1)
> +		die(_("options '%s', '%s', '%s', and '%s' cannot be used together"),
> +		    "-b", "-B", "--orphan", "--detach");
> +	if (opts.orphan && opt_track)
> +		die(_("'%s' and '%s' cannot be used together"), "--orphan", "--track");
> +	if (opts.orphan && !opts.checkout)
> +		die(_("'%s' and '%s' cannot be used together"), "--orphan",
> +		    "--no-checkout");
> +	if (opts.orphan && ac == 2)
> +		die(_("'%s' and '%s' cannot be used together"), "--orphan",
> +		    _("<commit-ish>"));
>   	if (lock_reason && !keep_locked)
>   		die(_("the option '%s' requires '%s'"), "--reason", "--lock");
>   	if (lock_reason)
> @@ -663,7 +702,9 @@ static int add(int ac, const char **av, const char *prefix)
>   		strbuf_release(&symref);
>   	}
> 
> -	if (ac < 2 && !new_branch && !opts.detach) {
> +	if (opts.orphan) {
> +		new_branch = orphan_branch;
> +	} else if (ac < 2 && !new_branch && !opts.detach) {
>   		const char *s = dwim_branch(path, &new_branch);
>   		if (s)
>   			branch = s;
> @@ -686,7 +727,11 @@ static int add(int ac, const char **av, const char *prefix)
>   	if (!opts.quiet)
>   		print_preparing_worktree_line(opts.detach, branch, new_branch, !!new_branch_force);
> 
> -	if (new_branch) {
> +	if (opts.orphan) {
> +		branch = new_branch;
> +	} else if (!lookup_commit_reference_by_name(branch)) {
> +		die(_("invalid reference: %s"), branch);
> +	} else if (new_branch) {
>   		struct child_process cp = CHILD_PROCESS_INIT;
>   		cp.git_cmd = 1;
>   		strvec_push(&cp.args, "branch");
> diff --git a/t/t2400-worktree-add.sh b/t/t2400-worktree-add.sh
> index dd729a00d8..45f896dcd0 100755
> --- a/t/t2400-worktree-add.sh
> +++ b/t/t2400-worktree-add.sh
> @@ -312,6 +312,11 @@ test_wt_add_excl () {
>   test_wt_add_excl -b poodle -B poodle bamboo main
>   test_wt_add_excl -b poodle --detach bamboo main
>   test_wt_add_excl -B poodle --detach bamboo main
> +test_wt_add_excl -B poodle --orphan poodle bamboo
> +test_wt_add_excl -b poodle --orphan poodle bamboo
> +test_wt_add_excl --orphan poodle --detach bamboo
> +test_wt_add_excl --orphan poodle --no-checkout bamboo
> +test_wt_add_excl --orphan poodle bamboo main
> 
>   test_expect_success '"add -B" fails if the branch is checked out' '
>   	git rev-parse newmain >before &&
> @@ -333,6 +338,46 @@ test_expect_success 'add --quiet' '
>   	test_must_be_empty actual
>   '
> 
> +test_expect_success '"add --orphan"' '
> +	test_when_finished "git worktree remove -f -f orphandir" &&
> +	git worktree add --orphan neworphan orphandir &&
> +	echo refs/heads/neworphan >expected &&
> +	git -C orphandir symbolic-ref HEAD >actual &&
> +	test_cmp expected actual
> +'
> +
> +test_expect_success '"add --orphan" fails if the branch already exists' '
> +	test_when_finished "git branch -D existingbranch" &&
> +	test_when_finished "git worktree remove -f -f orphandir" &&
> +	git worktree add -b existingbranch orphandir main &&
> +	test_must_fail git worktree add --orphan existingbranch orphandir2 &&
> +	test_path_is_missing orphandir2
> +'
> +
> +test_expect_success '"add --orphan" with empty repository' '
> +	test_when_finished "rm -rf empty_repo" &&
> +	echo refs/heads/newbranch >expected &&
> +	GIT_DIR="empty_repo" git init --bare &&
> +	git -C empty_repo  worktree add --orphan newbranch worktreedir &&
> +	git -C empty_repo/worktreedir symbolic-ref HEAD >actual &&
> +	test_cmp expected actual
> +'
> +
> +test_expect_success '"add" worktree with orphan branch and lock' '
> +	git worktree add --lock --orphan orphanbr orphan-with-lock &&
> +	test_when_finished "git worktree unlock orphan-with-lock || :" &&
> +	test -f .git/worktrees/orphan-with-lock/locked
> +'
> +
> +test_expect_success '"add" worktree with orphan branch, lock, and reason' '
> +	lock_reason="why not" &&
> +	git worktree add --detach --lock --reason "$lock_reason" orphan-with-lock-reason main &&
> +	test_when_finished "git worktree unlock orphan-with-lock-reason || :" &&
> +	test -f .git/worktrees/orphan-with-lock-reason/locked &&
> +	echo "$lock_reason" >expect &&
> +	test_cmp expect .git/worktrees/orphan-with-lock-reason/locked
> +'
> +
>   test_expect_success 'local clone from linked checkout' '
>   	git clone --local here here-clone &&
>   	( cd here-clone && git fsck )
> @@ -449,6 +494,14 @@ setup_remote_repo () {
>   	)
>   }
> 
> +test_expect_success '"add" <path> <remote/branch> w/ no HEAD' '
> +	test_when_finished rm -rf repo_upstream repo_local foo &&
> +	setup_remote_repo repo_upstream repo_local &&
> +	git -C repo_local config --bool core.bare true &&
> +	git -C repo_local branch -D main &&
> +	git -C repo_local worktree add ./foo repo_upstream/foo
> +'
> +
>   test_expect_success '--no-track avoids setting up tracking' '
>   	test_when_finished rm -rf repo_upstream repo_local foo &&
>   	setup_remote_repo repo_upstream repo_local &&
> --
> 2.38.2
> 
> 

>> % git -C bar.git worktree add --orphan main main/
>
> It's perhaps a bit late to bring this up but I've only just realized
> that it is unfortunate that --orphan takes a branch name rather than
> working in conjunction with -b/-B. It means that in the common case
> where the branch name is the same as the worktree the user has to
> repeat it on the command line as shown above. It also means there is
> no way to force an orphan branch (that's admittedly quite niche). If
> instead --orphan did not take an argument we could have
>
> 	git worktree add --orphan main
> 	git worktree add --orphan -b topic main
> 	git worktree add --orphan -B topic main

> Hi Jacob
>
> On 09/01/2023 17:33, Jacob Abel wrote:
> > [...]
>
> It's perhaps a bit late to bring this up but I've only just realized
> that it is unfortunate that --orphan takes a branch name rather than
> working in conjunction with -b/-B. It means that in the common case
> where the branch name is the same as the worktree the user has to repeat
> it on the command line as shown above. It also means there is no way to
> force an orphan branch (that's admittedly quite niche). If instead
> --orphan did not take an argument we could have
>
> 	git worktree add --orphan main
> 	git worktree add --orphan -b topic main
> 	git worktree add --orphan -B topic main
>
> Best Wishes
>
> Phillip
>
> > [...]

>> 	git worktree add --orphan -b topic main
>> 	git worktree add --orphan -B topic main
>
> I am hesitant to add these as they break away from the syntax used in
> `git switch` and `git checkout`.

>Jacob Abel <jacobabel@nullpo.dev> writes:
>
>>> 	git worktree add --orphan -b topic main
>>> 	git worktree add --orphan -B topic main
>>
>> I am hesitant to add these as they break away from the syntax used in
>> `git switch` and `git checkout`.
>
>Not that I care too deeply, but doesn't it introduce end-user confusion if we try to
>be compatible with "git checkout --orphan <branch>", while allowing this to be
>compatible with the default choice of the branch name done by "git worktree
>add"?  "--orphan" in "git checkout" behaves similar to "-b|-B" in that it always
>wants a name, but "git worktree add" wants to make it optional.
>
>By the way "--orphan" in checkout|switch wants to take a name for itself, e.g.
>
>	git checkout --orphan $name [$commit]
>	git checkout -b $name [$commit]
>	git checkout -B $name [$commit]
>
>so it is impossible to force their "--orphan" to rename an existing branch, which is
>probalby a design mistake we may want to fix.
>
>In any case, as I said, I do not care too deeply which way you guys decide to go,
>because I think the whole "orphan" UI is a design mistake that instills a broken
>mental model to its users [*].
>
>But let's wait a bit more to see which among
>
>(1) git worktree add [[--orphan] -b $branch] $path
>    This allows --orphan to act as a modifier to existing -b,
>
>(2) git worktree add [(--orphan|-b) $branch] $path
>    This allows --orphan to be another mode of -b, or
>
>(3) git worktree add [--orphan [$branch]|(-b $branch)] $path
>    This allows --orphan to default to $(basename $path)
>
>people prefer.
>
>
>[Footnote]
>
>* I am not saying that it is wrong or useless to keep an unrelated
>  history, especially one that records trees that have no relevance
>  to the main history like created with "switch --orphan", in the
>  same repository.  Allowing "git switch --orphan" to create such a
>  separate history in the same repository blurs the distinction.  It
>  would help newbies to form the right mental model if they start a
>  separate repository that the separate history originates in, and
>  pull from it to bootstrap the unrelated history in the local
>  repository.

> <rsbecker@nexbridge.com> writes:
>
> > [...]
>
> An orphan is not even detached, if I understand correctly.
>
> The state is what is called "being on an unborn branch", where your
> HEAD does not even point at any commit.  HEAD only knows a name of a
> branch that is not yet created but will be when you make a commit.
>
> While "(HEAD) being detached" means that you are on an existing
> commit---it is just that future history you extend by making a
> commit from that state will not be on any branch.
>
> So if we wanted to fix the misnomer, s/orphan/unborn/ would be how I
> would go about it.
>

> Jacob Abel <jacobabel@nullpo.dev> writes:
>
> >> 	git worktree add --orphan -b topic main
> >> 	git worktree add --orphan -B topic main
> >
> > I am hesitant to add these as they break away from the syntax used in
> > `git switch` and `git checkout`.
>
> Not that I care too deeply, but doesn't it introduce end-user
> confusion if we try to be compatible with "git checkout --orphan
> <branch>", while allowing this to be compatible with the default
> choice of the branch name done by "git worktree add"?  "--orphan" in
> "git checkout" behaves similar to "-b|-B" in that it always wants a
> name, but "git worktree add" wants to make it optional.

>
> By the way "--orphan" in checkout|switch wants to take a name for
> itself, e.g.
>
> 	git checkout --orphan $name [$commit]
> 	git checkout -b $name [$commit]
> 	git checkout -B $name [$commit]
>
> so it is impossible to force their "--orphan" to rename an existing
> branch, which is probalby a design mistake we may want to fix.

>
> But let's wait a bit more to see which among
>
> (1) git worktree add [[--orphan] -b $branch] $path
>     This allows --orphan to act as a modifier to existing -b,
>
> (2) git worktree add [(--orphan|-b) $branch] $path
>     This allows --orphan to be another mode of -b, or
>
> (3) git worktree add [--orphan [$branch]|(-b $branch)] $path
>     This allows --orphan to default to $(basename $path)
>
> people prefer.
>

>
> [Footnote]
>
> * I am not saying that it is wrong or useless to keep an unrelated
>   history, especially one that records trees that have no relevance
>   to the main history like created with "switch --orphan", in the
>   same repository.  Allowing "git switch --orphan" to create such a
>   separate history in the same repository blurs the distinction.  It
>   would help newbies to form the right mental model if they start a
>   separate repository that the separate history originates in, and
>   pull from it to bootstrap the unrelated history in the local
>   repository.

> On 23/01/14 07:09PM, Junio C Hamano wrote:
>> Jacob Abel <jacobabel@nullpo.dev> writes:
>>
>> >> 	git worktree add --orphan -b topic main
>> >> 	git worktree add --orphan -B topic main
>> >
>> > I am hesitant to add these as they break away from the syntax used in
>> > `git switch` and `git checkout`.
>>
>> Not that I care too deeply, but doesn't it introduce end-user
>> confusion if we try to be compatible with "git checkout --orphan
>> <branch>", while allowing this to be compatible with the default
>> choice of the branch name done by "git worktree add"?  "--orphan" in
>> "git checkout" behaves similar to "-b|-B" in that it always wants a
>> name, but "git worktree add" wants to make it optional.
>
> Yes. I think it's a fairly minor degree of confusion but I agree that it adds
> potentially unneeded confusion.

> On 23/01/13 10:20AM, Phillip Wood wrote:
>> Hi Jacob
>>
>> On 09/01/2023 17:33, Jacob Abel wrote:
>>> [...]
>>
>> It's perhaps a bit late to bring this up but I've only just realized
>> that it is unfortunate that --orphan takes a branch name rather than
>> working in conjunction with -b/-B. It means that in the common case
>> where the branch name is the same as the worktree the user has to repeat
>> it on the command line as shown above. It also means there is no way to
>> force an orphan branch (that's admittedly quite niche). If instead
>> --orphan did not take an argument we could have
>>
>> 	git worktree add --orphan main
>> 	git worktree add --orphan -b topic main
>> 	git worktree add --orphan -B topic main
>>
>> Best Wishes
>>
>> Phillip
>>
>>> [...]
> 
> I think this is a good idea and something similar was brought up previously
> however I originally wanted to handle this and a common --orphan DWYM in a later
> patch.

>> 	git worktree add --orphan main
> 
> I am OK implementing this option and have been workshopping it prior to
> responding. I think I have it worked out now as an additional patch which can be
> be applied on top of the v8 patchset.
> 
> I'll reply to this message with the one-off patch to get feedback. Since this is
> essentially a discrete change on top of v8, I can either keep it as a separate
> patch or reroll depending on how much needs to be changed (and what would be
> easier for everyone).
> 
>> 	git worktree add --orphan -b topic main
>> 	git worktree add --orphan -B topic main
> 
> I am hesitant to add these as they break away from the syntax used in
> `git switch` and `git checkout`.

> Also apologies for the tangent but while researching this path, I noticed that
> --orphan behaves unexpectedly on both `git switch` and `git checkout` when mixed
> with `-c` and `-b` respectively.
> 
>      % git switch --orphan -c foobar
>      fatal: invalid reference: foobar
> 
>      % git switch -c --orphan foobar
>      fatal: invalid reference: foobar
> >      % git checkout -b --orphan foobar
>      fatal: 'foobar' is not a commit and a branch '--orphan' cannot be created from it
>
>      % git checkout --orphan -b foobar
>      fatal: 'foobar' is not a commit and a branch '-b' cannot be created from it

> I tried this on my system install as well as from a fresh fetch of next FWIW.
> 
> [Info: fresh build from next]
> git version 2.39.0.287.g8cbeef4abd
> cpu: x86_64
> built from commit: 8cbeef4abda4907dd68ea144d9dcb85f0b49c3e6
> sizeof-long: 8
> sizeof-size_t: 8
> shell-path: /bin/sh
> 
> [Info: system install]
> git version 2.38.2
> cpu: x86_64
> no commit associated with this build
> sizeof-long: 8
> sizeof-size_t: 8
> shell-path: /bin/sh
> 
> If this bug is something that needs to be addressed, I can dig a bit deeper and
> put together a patch for it in the next few days.
> 
> VR,
> Abel
> 

> Hi Jacob
>
> On 14/01/2023 22:47, Jacob Abel wrote:
> > On 23/01/13 10:20AM, Phillip Wood wrote:
> >> Hi Jacob
> >>
> > [...]
> >
> > I'll reply to this message with the one-off patch to get feedback. Since this is
> > essentially a discrete change on top of v8, I can either keep it as a separate
> > patch or reroll depending on how much needs to be changed (and what would be
> > easier for everyone).
> >
> >> 	git worktree add --orphan -b topic main
> >> 	git worktree add --orphan -B topic main
> >
> > I am hesitant to add these as they break away from the syntax used in
> > `git switch` and `git checkout`.
>
> When I wrote my original email I wrongly though that --orphan did not
> take an argument for "git checkout". While I think it is a mistake for
> checkout and switch to have --orphan take an argument they do at least
> always need a branch name with that option. "git worktree" add already
> has the branch name in the form of the worktree directory in the common
> case.

>
> > Also apologies for the tangent but while researching this path, I noticed that
> > --orphan behaves unexpectedly on both `git switch` and `git checkout` when mixed
> > with `-c` and `-b` respectively.
> >
> >      % git switch --orphan -c foobar
> >      fatal: invalid reference: foobar
> >
> >      % git switch -c --orphan foobar
> >      fatal: invalid reference: foobar
> >      % git checkout -b --orphan foobar
> >      fatal: 'foobar' is not a commit and a branch '--orphan' cannot be created from it
> >
> >      % git checkout --orphan -b foobar
> >      fatal: 'foobar' is not a commit and a branch '-b' cannot be created from it
>
> The messages for checkout look better than the switch ones to me as they
> show the branch name which makes it clearer that we're treating what
> looks like an option as an argument. What in particular is unexpected
> here - --orphan and -b take an argument so they'll hoover up the next
> thing on the commandline whatever it is.
>
> Best Wishes
>
> Phillip
>
> > [...]

> On 23/01/16 10:47AM, Phillip Wood wrote:
>> Hi Jacob
>>
>> On 14/01/2023 22:47, Jacob Abel wrote:
>>> On 23/01/13 10:20AM, Phillip Wood wrote:
>>>> Hi Jacob
>>>>
>>> [...]
>>>
>>> I'll reply to this message with the one-off patch to get feedback. Since this is
>>> essentially a discrete change on top of v8, I can either keep it as a separate
>>> patch or reroll depending on how much needs to be changed (and what would be
>>> easier for everyone).
>>>
>>>> 	git worktree add --orphan -b topic main
>>>> 	git worktree add --orphan -B topic main
>>>
>>> I am hesitant to add these as they break away from the syntax used in
>>> `git switch` and `git checkout`.
>>
>> When I wrote my original email I wrongly though that --orphan did not
>> take an argument for "git checkout". While I think it is a mistake for
>> checkout and switch to have --orphan take an argument they do at least
>> always need a branch name with that option. "git worktree" add already
>> has the branch name in the form of the worktree directory in the common
>> case.
> 
> Understood.
> 
> I'm not entirely opposed to making this change to OPT_BOOL but I have to wonder
> how often `--orphan` will actually be used by a given user and whether the
> slightly shorter invocation will be used regularly.
> 
> With the base `git worktree add $path`, the shorthand/DWYM makes sense as it's
> used regularly but I don't see users working with `--orphan` outside of trying
> to create the first branch in a repository.

> And I'd like that operation of creating the first branch in a repo to eventually
> "just work" with the base command, i.e. `git worktree add main/`. The reason I
> hadn't yet added that is because I've yet to figure out how to get it to work
> without accidentally introducing potentially confusing situations and I didn't
> want to hold up introducing the core functionality itself.
> 
> Once that main use-case "just works", I don't see users utilising `--orphan`
> except in very rare circumstances. Doubly so since the average user likely
> shouldn't be using `--orphan` in most cases.

> Hence the question of whether this change would be worth it vs the existing
> `--orphan $branchname $path` which is (for better or worse) consistent with `-b`
> and `-B`.
> 
>>
>>> Also apologies for the tangent but while researching this path, I noticed that
>>> --orphan behaves unexpectedly on both `git switch` and `git checkout` when mixed
>>> with `-c` and `-b` respectively.
>>>
>>>       % git switch --orphan -c foobar
>>>       fatal: invalid reference: foobar
>>>
>>>       % git switch -c --orphan foobar
>>>       fatal: invalid reference: foobar
>>>       % git checkout -b --orphan foobar
>>>       fatal: 'foobar' is not a commit and a branch '--orphan' cannot be created from it
>>>
>>>       % git checkout --orphan -b foobar
>>>       fatal: 'foobar' is not a commit and a branch '-b' cannot be created from it
>>
>> The messages for checkout look better than the switch ones to me as they
>> show the branch name which makes it clearer that we're treating what
>> looks like an option as an argument. What in particular is unexpected
>> here - --orphan and -b take an argument so they'll hoover up the next
>> thing on the commandline whatever it is.
>>
>> Best Wishes
>>
>> Phillip
>>
>>> [...]
> 
> Agreed. I wasn't sure if this would be something worth addressing in a patch but
> at the very least I can work on putting together a small patch for `git switch`
> since it doesn't seem to be hoovering the flags like `git checkout` does.
> 

> On 18/01/2023 22:40, Jacob Abel wrote:
> > [...]
> > Understood.
> >
> > I'm not entirely opposed to making this change to OPT_BOOL but I have to wonder
> > how often `--orphan` will actually be used by a given user and whether the
> > slightly shorter invocation will be used regularly.
> >
> > With the base `git worktree add $path`, the shorthand/DWYM makes sense as it's
> > used regularly but I don't see users working with `--orphan` outside of trying
> > to create the first branch in a repository.
>
> Your example use in the commit message shows the user using the same
> name for the branch and worktree. If that really is the likely use than
> I think we should make --orphan OPT_BOOL. If it is not the likely use
> perhaps you could update the commit message to show how you think it
> will be used.

> > And I'd like that operation of creating the first branch in a repo to eventually
> > "just work" with the base command, i.e. `git worktree add main/`. The reason I
> > hadn't yet added that is because I've yet to figure out how to get it to work
> > without accidentally introducing potentially confusing situations and I didn't
> > want to hold up introducing the core functionality itself.
> >
> > Once that main use-case "just works", I don't see users utilising `--orphan`
> > except in very rare circumstances. Doubly so since the average user likely
> > shouldn't be using `--orphan` in most cases.
>
> This brings us back to the question that we discussed earlier of whether
> we need --orphan at all. If we can get the main use-case working without
> it we'd perhaps be better doing that rather than adding an option no one
> ends up using.

> Best Wishes
>
> Phillip
>
> > [...]

> While working with the worktree based git workflow, I realised that setting
> up a new git repository required switching between the traditional and
> worktree based workflows. Searching online I found a SO answer [1] which
> seemed to support this and which indicated that adding support for this should
> not be technically difficult.
>
> This patchset has four parts:
>   * adding `-B` to the usage docs (noticed during dev and it seemed too small
>     to justify a separate submission)
>   * adding a helper fn to simplify testing for mutual exclusion of options
>     in `t/t2400-worktree-add.sh`
>   * adding orphan branch functionality (as is present in `git-switch`)
>     to `git-worktree-add`
>   * adding an advise for using --orphan when `git worktree add` fails due to
>     a bad ref.
>
> Changes from v7:
>   * Changed test_wt_add_excl() to use `grep -E` instead of `grep -P` (2/4) [2][3].
>
> 1. https://stackoverflow.com/a/68717229/15064705/
> 2. https://lore.kernel.org/git/xmqq7cxxzefp.fsf@gitster.g/
> 3. https://lore.kernel.org/git/230109.86r0w328nu.gmgdl@evledraar.gmail.com/

> I've looked this & previous iterations over carefully, and this now
> looks good to me. Thanks for sticking with this so long.
>
> For what it's worth (if Junio would like to add it):
>
> Reviewed-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>