Re: PATCH: improve git switch documentation

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Martin wrote:
> On 09/07/2021 02:45, Felipe Contreras wrote:
> > I believe we have all the semantic tools needed to write something that
> > is understandable by most people regardless of their conception of what a
> > branch is.

> On 01/07/2021 16:58, Junio C Hamano proposed a patch that had an 
> interesting point.
> The patch was for the docs of "git switch" and "git branch"
> 
> 1)
> 
>       <start-point> versus <commit[-ish]>
> 
> I am not sure that this will help much with the original issue, which is 
> my concern that a (new) user will be aware of why "switch -C" is a force 
> (i.e. what the dangers are).
> But it is an interesting point.

I don't think it's an improvement. What is that <commitish> used for?
That's what the user wants to know, not to mention that not any commit
works.

>  From the synopsis of various commands (just a sample, I did not check all).
>    git switch (-c|-C) <new-branch> [<start-point>]
>    git branch <branchname> [<start-point>]
>    git checkout [--detach] <commit>
>    git checkout [[-b|-B|--orphan] <new_branch>] [<start_point>]
>    git reset [--soft | --mixed | --hard ] [<commit>]
> 
> With the exception for "git reset" they all use <start-point> when it 
> comes to branches.
> 
> The general question here is, should the synopsis say
> a) this parameter should be a "commit".
> And then the doc explains the commit will be used as startpoint

I'd say no. I think it's pretty obvious what these commands accept as
input, what isn't clear is what that input is for.

> b) this parameter should be a "start point"
> And then the doc explains the startpoint has to be given as commit.

I don't see much value in explaining that has to be given as a commit.
How else would it be given as?

> In terms of checkout, this is especially interesting.
> The 2nd form does create a new branch.

You could say both forms create a new branch, except in the first form
the branch doesn't have a name.

> But both forms check-out the commit.
> IMHO it is somewhat strange that you "check out a start-point to your 
> worktree".
> 
> So probably <commit> (or even <commit-ish>) may indeed be the better option.

But we don't need all the commands to say the same thing, what we need
is something that's easy for the user to understand, and it's accurate.

> This is however an issue that goes well beyond "git switch".

Indeed, but if history is an indication nothing will change (changes in
git's UI rarely do happen), so its better to minimize the possibility
that the patch will be ignored, or straight up rejected.

So it's better to stick with the experimental command and fix that
first.

> This may also affect other words used in synopsises. So this is a 
> general rule that needs to be decided for all of the documentation.
> The issue is, that some commands take several commits.
>     git rebase [--onto <newbase>] [<upstream> [<branch>]]
> In that case some distinguishing is needed.

I'd say it shouldn't matter if it recevies one or several, what that
commit is used for is what matters.

> 2)
> 
>       <branch> versus <branch-name>
> 
>     git switch [--no-guess] <branch>
>     git switch (-c|-C) <new-branch> [<start-point>]
>     git branch <branchname> [<start-point>]
>     git checkout [[-b|-B|--orphan] <new_branch>] [<start_point>]
>     git rebase [--onto <newbase>] [<upstream> [<branch>]]
> 
> First of all "git rebase" is simply wrong. I can give a commit for all 3 
> arguments. So the last one does not have to be a branch. (or <branch-name>)

True. Although in most cases the last one would be a branch.

> Then I think <branch-name> (or <branch-ish> /see other mail) should be 
> preferred over <branch>.

I don't think it makes a difference. A branch name is how you refer to a
branch (what else would be there?).

Differentiating the difference between a branch and a branch name was
done to write better sentences in the description of what the commands
do, but in the synopsis I don't see what we gain.

> As for "git switch -C"
> This should IMHO change to (the 2nd arg, actually depends on the point 
> "1" above)
>     git switch (-c|-C) <branch-name> [<base-commit>]
> 
> I suggest to not call it "new-branch-name" because, it might be an 
> existing name.

I think the name is all wrong. As Ævar pointed out --new (-n) is much
better. Also it doesn't make much sense to use "create" or "new" for
something that already exists.

I think you saw a correct issue: `git switch -C` might be used
incorrectly, but changing to the documentation would have limited value
(and only for the ones that read it).

I think if the branch already exists, the user has to be explicit to
what he wants to do and use `git switch --reset <branch> <commit>`

> 3)
> 
>     newbbranch  versus new-branch  versus  new_branch
> 
> That is something that just needs to be decided.
> "new_branch" is in git checkout.

I'd rather have <branch>, but as I already said, the more ground you try
to cover the more impossible it will be to actually land the changes.

> 4)
> 
>     Extend of explanation for why a command is classified as "force".
> 
> This one is the one I still lobby for support.
> This is also on issue across all docs. (or most)
> 
> Currently "git switch -C" is simply stated to be --force-create.
> 
> - There is no mention what is "forced". All it says is:
> >  if <new-branch> already exists, it will be reset to <start-point>.
> I guess this is the English verb reset. Because, if the user goes to 
> "git reset" then the user would not know what kind of reset.
> So the term "reset" is ambiguous, as it could be the verb, or the command.
> 
> Of course the "git branch" doc has the same
> > Reset <branchname> to <startpoint>, even if <branchname> exists already. 
> 
> 
> There is also no word, that this does not include overwriting a dirty 
> work tree.
> 
>    git switch --force -c unused-name origin/branch
> means "forcefully overwrite a dirty work tree"
> 
>    git switch --force-create unused-name origin/branch
> fails on the dirty work tree.
> 
> 
> Btw similar on "git checkout"
>    git checkout -B unused-name origin/branch
> Only difference, -B has no misleading long option.
> 
> 
> But my point is less, the not applying danger.
> My point is what danger is there, so that this was made a force command?
> 
> Look at
>    git checkout --force
> > --force
> >     When switching branches, proceed even if the index or the working tree differs from HEAD. This is used to throw away local changes.
> 
>    git switch --force
> > --force
> >     An alias for --discard-changes.
> and then eventually
> > This is used to throw away local changes.
> 
> So --force clearly says: You will loose local changes (if you have any).
> 
> The same clarity is missing for "force create branch".
> 
> Yes, sure any commits that where in the branch, may be hold by other 
> branches or the ref-log.
> But neither is guaranteed. A branch does not need to have a reflog.
> 
> Even if we say a user must know about certain concepts (such as a 
> branchname is a reference, and non referenced objects may be lost), even 
> then the user is left to connect the dots themself.
> 
> I think it should be included in the docs (git switch/checkout/branch 
> and reset)
> 
> The current wording
>     Reset <branchname> to <startpoint>, even if
>     <branchname> exists already.
> 
> should be amended
> Avoid "reset"
>     Create a new branch at <startpoint> with the name
>     <branchname>, even if <branchname> is already used.
> 
> Add clarity
>     Create a new branch at <startpoint> with the name
>     <branchname>.
>     If <branchname> already existed, then the old branch
>     will be removed.
> 
> If the user perceives "the old branch" as container for a "chain of 
> commits", then it is still up to the user to know, that any of those 
> commits can be part of other branches. And that "removing the branch", 
> may or may not include removing the commits.
> 
> However, a user not yet knowing what exactly "removing a branch" means, 
> does at least have the word "remove" to make him wary that they should 
> look up more details.

All these issues go away if we have:

  git switch --reset <branch> <commit>

And instead of -C, we have:

  git switch --new --reset <branch> <commit>

This creates a new branch if it doesn't exist, or if it exists resets
it.

Now the documentation writes itself.

Cheers.

-- 
Felipe Contreras



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux