From: Elijah Newren <newren@gmail.com>
Date: Mon, 11 Mar 2019 17:54:16 GMT
Subject: Re: [PATCH v3 10/21] checkout: split part of it to new command 'switch'
Message-ID: <CABPp-BEBudobnduipQrSvyQWQ7Hb4WmmoptCQ+FyY8BRPp7_ZA@mail.gmail.com>
In-Reply-To: <20190308095752.8574-11-pclouds@gmail.com>

A few other comments that I thought of while responding elsewhere in
the thread that didn't make sense to include elsewhere...

On Fri, Mar 8, 2019 at 2:00 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
> something-to-checkout-paths. The good old "git checkout" command is

s/something-to-checkout-paths/restore/ ?

> +-g::
> +--guess::
> +       If `<branch>` is not found but there does exist a tracking
> +       branch in exactly one remote (call it `<remote>`) with a
> +       matching name, treat as equivalent to
> ++
> +------------
> +$ git switch -c <branch> --track <remote>/<branch>
> +------------

I'm not sure if it'd be better or worse to simplify this to
   git switch --track <remote>/<branch>
Thoughts?

> ++
> +If the branch exists in multiple remotes and one of them is named by
> +the `checkout.defaultRemote` configuration variable, we'll use that
> +one for the purposes of disambiguation, even if the `<branch>` isn't
> +unique across all remotes. Set it to e.g. `checkout.defaultRemote=origin`
> +to always checkout remote branches from there if `<branch>` is
> +ambiguous but exists on the 'origin' remote. See also
> +`checkout.defaultRemote` in linkgit:git-config[1].
> +
> +-q::
> +--quiet::
> +       Quiet, suppress feedback messages.

--quiet and --progress should be adjacent in the man-page since they
are touching upon the same concept.  It'd be nice if we had a way to
group similar items like this besides simply putting them together,
but we can at least do that.

> +
> +-f::
> +--force::
> +       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.

--force (or --discard-changes as mentioned elsewhere in this thread),
-m/--merge, and --conflict are also similar options and should be
adjacent in the man-page; they all reflect on how to handle switching
when local changes affect the same paths that different between HEAD
and the other branch.

> +--progress::
> +--no-progress::
> +       Progress status is reported on the standard error stream
> +       by default when it is attached to a terminal, unless `--quiet`
> +       is specified. This flag enables progress reporting even if not
> +       attached to a terminal, regardless of `--quiet`.

This again makes me curious what --quiet actually supresses; in the
case of branch switching, are there any non-warning informational
messages other than progress reports that are printed?  If not, the
extra sentence at the end of the description for --progress can (and
probably should) be stripped.

> +
> +-t::
> +--track::
> +       When creating a new branch, set up "upstream" configuration.
> +       `-c` is implied. See "--track" in linkgit:git-branch[1] for
> +       details.
> ++
> +If no `-c` option is given, the name of the new branch will be derived
> +from the remote-tracking branch, by looking at the local part of the
> +refspec configured for the corresponding remote, and then stripping
> +the initial part up to the "*".  This would tell us to use "hack" as
> +the local branch when branching off of "origin/hack" (or
> +"remotes/origin/hack", or even "refs/remotes/origin/hack").  If the
> +given name has no slash, or the above guessing results in an empty
> +name, the guessing is aborted.  You can explicitly give a name with
> +`-c` in such a case.

Slightly disappointed that we couldn't remove --track; it has always
seemed a bit complicated.  But I don't see how to do so, since people
do want upstream branches to be tracked with their new branches.

> +
> +--no-track::
> +       Do not set up "upstream" configuration, even if the
> +       branch.autoSetupMerge configuration variable is true.
> +
> +-m::
> +--merge::
> +       If you have local modifications to one or more files that are
> +       different between the current branch and the branch to which
> +       you are switching, the command refuses to switch branches in
> +       order to preserve your modifications in context.  However,
> +       with this option, a three-way merge between the current
> +       branch, your working tree contents, and the new branch is
> +       done, and you will be on the new branch.
> ++
> +When a merge conflict happens, the index entries for conflicting
> +paths are left unmerged, and you need to resolve the conflicts
> +and mark the resolved paths with `git add` (or `git rm` if the merge
> +should result in deletion of the path).

Now that Phillip highlighted issues with -m and -f, it's hard not to
wonder about other corner cases.  For example, what if the user made
some changes, staged them, then made more changes, then tried to 'git
checkout -m <other branch>'?  That's no longer a three-way merge, but
four way.  How does that work?  Does it just rely on merge-recursive's
(poorly defined) choice of when to bail out and when to permit such
craziness?

> +--conflict=<style>::
> +       The same as --merge option above, but changes the way the
> +       conflicting hunks are presented, overriding the
> +       merge.conflictStyle configuration variable.  Possible values are
> +       "merge" (default) and "diff3" (in addition to what is shown by
> +       "merge" style, shows the original contents).
> +
> +--orphan <new-branch>::
> +       Create a new 'orphan' branch, named `<new-branch>`, started from
> +       `<start-point>` and switch to it. See explanation of the same
> +       option in linkgit:git-checkout[1] for details.

Sigh...does this mean --orphan will remain broken?  It has always
driven me crazy that it leaves you with a fully populated rather than
an empty index.  It seemed broken to me before I figured out the
special usecase, though it still seemed like the wrong default (an
empty index wouldn't surprise due to the "orphan" name, but a full one
does to those without the special usecase in mind).  Oh well, that's a
much smaller battle than the big picture of getting switch and restore
in place, and I don't want to derail the bigger picture; anything
using --orphan is a somewhat special case anyway.

> +You can give the `-m` flag to the command, which would try a three-way
> +merge:
> +
> +------------
> +$ git switch -m mytopic
> +Auto-merging frotz
> +------------
> +
> +After this three-way merge, the local modifications are _not_
> +registered in your index file, so `git diff` would show you what
> +changes you made since the tip of the new branch.

...even if the local modifications were registered in the index file
before?  Is this why Phillip's "x" was missing and how it avoided
doing a four-way merge?  I guess it kinda makes sense, view from this
angle, but I'm not so sure I like it.  Hmm....