On 23-nov-2022 05:37:21, Jacob Abel wrote:
> 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@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")) {
> >
> > 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.
>
Understood. Maybe allowing a mixed DWYM...
$ git worktree add --orphan foobar
I'll wait for your next version.
Thank you for the wrap up, and sorry again for reading the thread bottom
up.
