On 22/11/22 12:16AM, Eric Sunshine wrote:
> On Sat, Nov 19, 2022 at 6:49 AM Ævar Arnfjörð Bjarmason
> <avarab@xxxxxxxxx> 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);
+ 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");
>
> 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).
Will do.
>
> > > 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.
Perfect. I'll work on getting the next revision for the patchset out then.
