Re: PATCH: improve git switch documentation

From: Junio C Hamano <gitster@pobox.com>
Date: Jun 29, 2021, 16:35 UTC
Message-ID: <xmqqk0mcy6g2.fsf@gitster.g>
In-Reply-To: <c593a699-eaf2-c7ab-b522-bfd224fce829@mfriebe.de>

Martin <git@mfriebe.de> writes:

Show 19 quoted lines

> Below is a patch, that I believe would improve the documentation of
> git switch.
>
> The exact new wording is of course open for debate.
>
> Reasoning for the change.
>
> The current doc does not explain why the option is a "forceful" option.
> Nor does explain the consequences.
>
> Instead it leaves it to the user to lookup the alternate command, and
> find the meaning of
>     git branch -f newbranch
>
> Only if the user does that successfully, the user may learn about the
> full consequences of their actions.
>
> I believe this info should be part of the "git switch" doc,
> itself. (Especially due to the severity that the action may have).

Please place all of the above below the three-dash line.

> From 46580d07f95a18c94925afd141ba55e52a82c8e1 Mon Sep 17 00:00:00 2001

Lose this line.

> From: Martin <User4martin@users.noreply.github.com>

Get rid of this line, too, as you have your own e-mail address on the real "From" line of the e-mail.

> Date: Tue, 29 Jun 2021 17:22:25 +0200

This too.

> Subject: [PATCH] Update git-switch.txt

And this one, too.

And then justify and describe the change here (see Documentation/SubmittingPatches::describe-changes)

Immediately before the three-dash line below, have your sign-off (see Documentation/SubmittingPatches::sign-off).

Show 16 quoted lines

> ---
>  Documentation/git-switch.txt | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/git-switch.txt b/Documentation/git-switch.txt
> index 5c438cd5058758..80acafad1f4a46 100644
> --- a/Documentation/git-switch.txt
> +++ b/Documentation/git-switch.txt
> @@ -70,8 +70,12 @@ $ git switch <new-branch>
>  -C <new-branch>::
>  --force-create <new-branch>::
>  	Similar to `--create` except that if `<new-branch>` already
> -	exists, it will be reset to `<start-point>`. This is a
> -	convenient shortcut for:
> +	exists, it will be reset to `<start-point>`.
> +	This forces the branch to the new location.

I would have written "This forces the branch to point at a different commit", as we do not have to use a fuzzy word "location" in this context (is it a location in the directory structure in the working tree? is it a location in the history dag? is it a location in some other dimension?).

Up to this point, it makes sense.

> + It also forces
> +	any commit hold by the branch to be dropped, unless the
> +	commit is also part of any other branch too. You may
> +	therefore loose some of your data.

Aside from typo on "lose" (not "loose") and "held" (not "hold"), this paragraph does not seem to add much value, at least to me, and I suspect that it makes things even more confusing to new readers.

 * Repointing the branch tip to a different commit is not limited to
   "git switch -C".  Any commands that allow you to move the branch
   tip, like "git branch -f", "git checkout -B", "git push --force",
   "git reset", share the same property and singling "switch -C" out
   gives a false impression that all other commands are OK.

 * "to be dropped" is unnecessarily alarming (and not even correct).
   "gc" will not reclaim while the reflog entries hold onto them.

   "Some commits that used to be reachable from the original branch
   tip may become unreachable." would not be an incorrect
   description per-se (and would be a vast improvement over what is
   in the posted patch), but it is dubious to stress the obvious,
   especially given that the whole point of "branch -f" is to make
   wrong commits disappear by pointing at corrected commits with the
   branch tip.

Because "switch -c <new-branch>", unlike "switch <existing-branch>" would not have to touch the working tree at all, the only reason why the user has to force the operation by using "-C" is to override the safety offered by "-c" that protects existing branches from accidental overwriting. Perhaps adding some description on "why" -c prevents an existing branch from being overwritten would help reduce the confusion better than an additional warning on "-C"?

Thanks.