Hi all, Sorry for dropping out of the conversation, but I see that the changes landed on the master, ready for the 2.44.0 release. Thank you very much! On Fri, 8 Dec 2023 at 18:13, Junio C Hamano <gitster@xxxxxxxxx> wrote: > > Eric Sunshine <sunshine@xxxxxxxxxxxxxx> writes: > > > Needs review and documentation updates. > > > > I'm not sure if the "Needs review" comment is still applicable since > > the patch did get some review comments, however, the mentioned > > documentation update is probably still needed for this series to > > graduate. > > Thanks. I think "-B" being defined as "branch -f <branch>" followed > by "checkout <branch>" makes it technically unnecessary to add any > new documentation (because "checkout <branch>" will refuse, so it > naturally follows that "checkout -B <branch>" should), but giving > the failure mode a bit more explicit mention would be more helpful > to readers. > > Here is to illustrate what I have in mind. The mention of the > "transactional" was already in the documentation for the "checkout" > back when switch was described at d787d311 (checkout: split part of > it to new command 'switch', 2019-03-29), but somehow was left out in > the documentation of the "switch". While it is not incorrect to say > that it is a convenient short-cut, it is more important to say what > happens when one of them fails, so I am tempted to port that > description over to the "switch" command, and give the "used elsewhere" > as a sample failure mode. > > The test has been also enhanced to check the "transactional" nature. > > Documentation/git-checkout.txt | 4 +++- > Documentation/git-switch.txt | 9 +++++++-- > t/t2400-worktree-add.sh | 18 ++++++++++++++++-- > 3 files changed, 26 insertions(+), 5 deletions(-) > > diff --git c/Documentation/git-checkout.txt w/Documentation/git-checkout.txt > index 240c54639e..55a50b5b23 100644 > --- c/Documentation/git-checkout.txt > +++ w/Documentation/git-checkout.txt > @@ -63,7 +63,9 @@ $ git checkout <branch> > ------------ > + > that is to say, the branch is not reset/created unless "git checkout" is > -successful. > +successful (e.g., when the branch is in use in another worktree, not > +just the current branch stays the same, but the branch is not reset to > +the start-point, either). > > 'git checkout' --detach [<branch>]:: > 'git checkout' [--detach] <commit>:: > diff --git c/Documentation/git-switch.txt w/Documentation/git-switch.txt > index c60fc9c138..6137421ede 100644 > --- c/Documentation/git-switch.txt > +++ w/Documentation/git-switch.txt > @@ -59,13 +59,18 @@ out at most one of `A` and `B`, in which case it defaults to `HEAD`. > -c <new-branch>:: > --create <new-branch>:: > Create a new branch named `<new-branch>` starting at > - `<start-point>` before switching to the branch. This is a > - convenient shortcut for: > + `<start-point>` before switching to the branch. This is the > + transactional equivalent of > + > ------------ > $ git branch <new-branch> > $ git switch <new-branch> > ------------ > ++ > +that is to say, the branch is not reset/created unless "git switch" is > +successful (e.g., when the branch is in use in another worktree, not > +just the current branch stays the same, but the branch is not reset to > +the start-point, either). > > -C <new-branch>:: > --force-create <new-branch>:: > diff --git c/t/t2400-worktree-add.sh w/t/t2400-worktree-add.sh > index bbcb2d3419..5d5064e63d 100755 > --- c/t/t2400-worktree-add.sh > +++ w/t/t2400-worktree-add.sh > @@ -129,8 +129,22 @@ test_expect_success 'die the same branch is already checked out' ' > test_expect_success 'refuse to reset a branch in use elsewhere' ' > ( > cd here && > - test_must_fail git checkout -B newmain 2>actual && > - grep "already used by worktree at" actual > + > + # we know we are on detached HEAD but just in case ... > + git checkout --detach HEAD && > + git rev-parse --verify HEAD >old.head && > + > + git rev-parse --verify refs/heads/newmain >old.branch && > + test_must_fail git checkout -B newmain 2>error && > + git rev-parse --verify refs/heads/newmain >new.branch && > + git rev-parse --verify HEAD >new.head && > + > + grep "already used by worktree at" error && > + test_cmp old.branch new.branch && > + test_cmp old.head new.head && > + > + # and we must be still on the same detached HEAD state > + test_must_fail git symbolic-ref HEAD > ) > ' >
