Re: [PATCH] worktree add: shouldn't dwim if -b or -B is given

On Wed, Aug 5, 2026 at 2:11 AM Junio C Hamano <[email protected]> wrote:
> Stepping back a bit, though, does this change the behavior when
> 'branch' *does* resolve to a commit (hence, the DWIM is already
> bypassed and 'new_branch' or 'branch' are not nuked)?  When 'ac' is
> equal to 2 and 'new_branch' is supplied, we used to call:
>
>         if (!strcmp(branch, "HEAD"))
>                 can_use_local_refs(&opts);
>
> inside the block you are now skipping.  It looks to me that this
> patch also changes behavior when the user says:
>
>         $ git worktree add -b <new-branch> <path> HEAD
>
> by not calling can_use_local_refs(), whose only effect in this
> context is that it may issue a warning() to the user.  I do not know
> offhand what the ramifications of this difference are.

It is better to emit warnings for the given arguments whenever possible,
so I think we should not skip the step mentioned above.
I'll modify the condition.

> > diff --git a/t/t2400-worktree-add.sh b/t/t2400-worktree-add.sh
> > index 87b926728a..9cbf84861d 100755
> > --- a/t/t2400-worktree-add.sh
> > +++ b/t/t2400-worktree-add.sh
> > @@ -621,6 +621,16 @@ test_expect_success '"add" <path> <branch> dwims' '
> >       )
> >  '
> >
> > +test_expect_success '"add" <path> <branch> does not dwim with -b' '
> > +     test_when_finished rm -rf repo_upstream repo_dwim foo &&
> > +     setup_remote_repo repo_upstream repo_dwim &&
> > +     git init repo_dwim &&
> > +     (
> > +             cd repo_dwim &&
> > +             test_must_fail git worktree add -b branch ../foo foo
> > +     )
> > +'
>
> Two comments.
>
>  * You call setup_remote_repo(), which takes two names, runs
>    'git init' on each of them, and prepares those repositories.  Do
>    you need to run 'git init repo_dwim' to initialize it again?
>    What for?

'git init' was unnecessary.  It was included because I copied
an existing test; I'll remove it.

>  * Expecting a failure to create a worktree may be fine, but can we
>    say how we expect the command to fail as well?  Perhaps you can
>    capture the error message while ensuring that the command fails,
>    and then check the error message to make sure it failed in the
>    expected way, or something like that.

Expected error is 'fatal: invalid reference: foo'. I'll add test_grep.

Thanks,
-- 
Yoichi NAKAYAMA