Unrelated: I found it confusing that my `co = checkout` alias did not work with this fresh-off-the-press Advice:[1] ``` $ ./git co master fatal: 'master' is already used by worktree at '<path>' $ ./git checkout master fatal: 'master' is already used by worktree at '<patch>' hint: Use --detach to avoid this restriction, hint: or --ignore-other-worktrees to ignore it. hint: Disable this message with "git config set advice.branchUsedInOtherWorktree false" ``` But it did for this older Advice (which is in my installed git(1)): ``` $ ./git co -b .. @ fatal: '..' is not a valid branch name hint: See `man git check-ref-format` hint: Disable this message with "git config set advice.refSyntax false" ``` It’s because aliases are run as a subprocess from the `git` in `PATH`: ``` strvec_push(&cmd.args, "git"); ``` [1]: Chain of events: 1. Try to trigger the Advice in this series 2. ... but it doesn’t 3. Is the code wrong? 4. Wait, I’m using my alias (which I always use; I don’t think about it) 5. I test with `git checkout`: it works 6. ... so aliases don’t work with Advice? 7. Test an existing Advice that I know about 8. ... but it does work with aliases 9.–15. ... It was part of the process. I didn’t *decide* to get hung up on it. ;) On Sat, Sep 13, 2025, at 16:13, Gabriel Scherer wrote: > From: "Gabriel.Scherer" <gabriel.scherer@xxxxxxxx> > > When checkout/switch fails because the target branch is already used > by another worktree, we now hint that the user could use --detach or > --ignore-other-worktrees. The commit message is supposed to discuss what the code does without the patch in the present tense. What the patch does is in the imperative mood. Refer to `Documentation/SubmittingPatches`, “imperative mood”. Maybe there should be a paragraph which motivates the need for an Advice and these two in particular? You could also concievably advise removing the worktree. :) > Note: this error can also happen on rebase, which unfortunately does > not support --ignore-other-worktrees. We do not show advice in this > case, and leave 'rebase --ignore-other-worktrees' to future work. After reading this I thought this was not handled by this patch series. But you add the this option to git-rebase(1) in the second patch. “Future work” suggests to me something ranging from: • there is another patch series that deal with this; or • there are no plans to do this (but it could be done in theory). > > Signed-off-by: Gabriel Scherer <gabriel.scherer@xxxxxxxx> > --- > Documentation/config/advice.adoc | 3 +++ > advice.c | 1 + > advice.h | 1 + > branch.c | 13 +++++++++++-- > branch.h | 4 ++++ > builtin/checkout.c | 12 ++++++++++-- > 6 files changed, 30 insertions(+), 4 deletions(-) > > diff --git a/Documentation/config/advice.adoc > b/Documentation/config/advice.adoc > index 257db58918..9ee64f44ea 100644 > --- a/Documentation/config/advice.adoc > +++ b/Documentation/config/advice.adoc > @@ -27,6 +27,9 @@ all advice messages. > Shown when a fetch refspec for multiple remotes maps to > the same remote-tracking branch namespace and causes branch > tracking set-up to fail. > + branchUsedInOtherWorktree:: > + Shown when the user attemps to switch to a branch > + that is already checked out in another worktree. This maintains sort-order which is good. It is also consistent in all points with 95c987e6fad (advice: make all entries stylistically consistent, 2024-03-05) for what it’s worth. I also think the wording/formulation is great. > checkoutAmbiguousRemoteBranchName:: > Shown when the argument to > linkgit:git-checkout[1] and linkgit:git-switch[1] > diff --git a/advice.c b/advice.c > index e5f0ff8449..5c9b763472 100644 > --- a/advice.c > +++ b/advice.c > @@ -50,6 +50,7 @@ static struct { > [ADVICE_AMBIGUOUS_FETCH_REFSPEC] = { "ambiguousFetchRefspec" }, > [ADVICE_AM_WORK_DIR] = { "amWorkDir" }, > [ADVICE_CHECKOUT_AMBIGUOUS_REMOTE_BRANCH_NAME] = { "checkoutAmbiguousRemoteBranchName" }, > + [ADVICE_BRANCH_USED_IN_OTHER_WORKTREE] = { "branchUsedInOtherWorktree" }, This too looks like it should be in sort order. If so you are inserting at the wrong place. > [ADVICE_COMMIT_BEFORE_MERGE] = { "commitBeforeMerge" }, > [ADVICE_DEFAULT_BRANCH_NAME] = { "defaultBranchName" }, > [ADVICE_DETACHED_HEAD] = { "detachedHead" }, > diff --git a/advice.h b/advice.h > index 727dcecf4a..6b11df945b 100644 > --- a/advice.h > +++ b/advice.h > @@ -16,6 +16,7 @@ enum advice_type { > ADVICE_ADD_IGNORED_FILE, > ADVICE_AMBIGUOUS_FETCH_REFSPEC, > ADVICE_AM_WORK_DIR, > + ADVICE_BRANCH_USED_IN_OTHER_WORKTREE, Correct sort order (if there is one). > ADVICE_CHECKOUT_AMBIGUOUS_REMOTE_BRANCH_NAME, > ADVICE_COMMIT_BEFORE_MERGE, > ADVICE_DEFAULT_BRANCH_NAME, > diff --git a/branch.c b/branch.c > index 26be358347..76aa2cbf44 100644 > --- a/branch.c > +++ b/branch.c > @@ -844,7 +844,7 @@ void remove_branch_state(struct repository *r, int > verbose) > remove_merge_branch_state(r); > } >[snip diff] > diff --git a/builtin/checkout.c b/builtin/checkout.c > index f9453473fe..e4b78f4a05 100644 > --- a/builtin/checkout.c > +++ b/builtin/checkout.c > @@ -1582,8 +1582,16 @@ static void > die_if_switching_to_a_branch_in_use(struct checkout_opts *opts, > return; > head_ref = refs_resolve_refdup(get_main_ref_store(the_repository), > "HEAD", 0, NULL, &flags); > - if (head_ref && (!(flags & REF_ISSYMREF) || strcmp(head_ref, > full_ref))) > - die_if_checked_out(full_ref, 1); > + if (head_ref && (!(flags & REF_ISSYMREF) || strcmp(head_ref, > full_ref))) { > + int code = die_message_if_checked_out(full_ref, 1); > + if (code) { > + advise_if_enabled( > + ADVICE_BRANCH_USED_IN_OTHER_WORKTREE, > + _("Use --detach to avoid this restriction,\n" > + "or --ignore-other-worktrees to ignore it.")); I don’t know if `--detach` will “avoid” the restriction. (In fact `--ignore-other-worktrees` might be the one that *avoids* it (turns it off)?) Technically the only point of being-on-a-branch is to be able to advance it. You know that. But does the advice-receiver? Because they might use the hint to get what they want immediately. Then later wonder why all the work they did on the branch “had no effect”. > + exit(code); > + } > + } > free(head_ref); > } > > -- > 2.51.0 No updates to tests were covered in another email. -- Kristoffer Haugsbakk