Re: [PATCH v3 0/2] worktree: Support `--orphan` when creating new worktrees

["Rubén Justo" <rjusto@gmail.com>]

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.  This needs to be "HEAD",
in capital letters.

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

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.

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;