Re: [PATCH v4 12/26] checkout: split part of it to new command 'switch'

[Elijah Newren <newren@gmail.com>]

On Sun, Mar 17, 2019 at 5:50 AM Nguyễn Thái Ngọc Duy <pclouds@gmail.com> wrote:
>
> "git checkout" doing too many things is a source of confusion for many
> users (and it even bites old timers sometimes). To remedy that, the
> command will be split into two new ones: switch and restore. The good
> old "git checkout" command is still here and will be until all (or most
> of users) are sick of it.
>
> See the new man page for the final design of switch. The actual
> implementation though is still pretty much the same as "git checkout"
> and not completely aligned with the man page. Following patches will
> adjust their behavior to match the man page.
>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---

Looking really good.  Just some minor comments...

> +git-switch(1)
> +=============
> +
> +NAME
> +----
> +git-switch - Switch branches
> +
> +SYNOPSIS
> +--------
> +[verse]
> +'git switch' [<options>] [--guess] <branch>

Should this now use [--no-guess] since --guess is the default?

> +'git switch' [<options>] --detach [<start-point>]
> +'git switch' [<options>] (-c|-C|--orphan) <new-branch> [<start-point>]
> +
> +DESCRIPTION
> +-----------
> +Switch to a specified branch. The working tree and the index are
> +updated to match the branch. All new commits will be added to the tip
> +of this branch.
> +
> +Optionally a new branch could be created with either `-c`, `-C`,
> +automatically from a remote branch of same name (see `--guess`), or
> +detach the working tree from any branch with `--detach`, along with
> +switching.
> +
> +Switching branches does not require a clean index and working tree
> +(i.e. no differences compared to `HEAD`). The operation is aborted
> +however if the switch leads to loss of local changes, unless told
> +otherwise.

Maybe s/otherwise./otherwise with --discard-changes or --merge./, just
for a little extra clarity?

> +-f::
> +--force::
> +       An alias for `--discard-changes`.
> +
> +--discard-changes::
> +       Proceed even if the index or the working tree differs from
> +       `HEAD`. Both the index and working tree are restored to match
> +       the switching target. This is used to throw away local
> +       changes.

It looks like elsewhere you and Junio discussed making --force also
imply --ignore-in-progress.  That option should be moved close to
--force, so that similar options are adjacent, but it also brings up a
question for me:

Is --force an alias for both `--discard-changes` and
`--ignore-in-progress`, or is `--discard-changes` really just another
name for `--force` (i.e. does it too imply `--ignore-in-progress`)?
I'd be tempted to say the former, but I'm curious on others' thoughts.

> +--orphan <new-branch>::
> +       Create a new 'orphan' branch, named `<new-branch>`. If
> +       `<start-point>` is specified, the index and working tree are
> +       adjusted to match it. Otherwise both are adjusted to contain no
> +       tracked files.

Thanks.  I'm still slightly hesitant about whether <start-point>
should be allowed with --orphan; it seems equivalent to me to letting
people have a flag for switching to existing branch A while forcing
the index and working tree to match branch B (defaulting B to HEAD
from before the switch).  Having <start-point> and --orphan together
is just a special case of this idea, and thus allowing those together
seems like it'll cause the more general request to be filed at some
point, and we will already have the precedent of supporting it
somewhere.  This usecase seems to be somewhat esoteric and infrequent,
and could be easily obtained by combining other commands.  I'm worried
that trying to explain this usecase may make the documentation for the
common everyday commands even more complex, and these manpages are
already kind of long.  However, as I said above, that's just a slight
hesitation and maybe I'm just excessively worried about the length of
our manpages for the most common commands.  This new description of
--orphan at least gets the default behavior right.


Great work so far; thanks for working on this.

Elijah