Re: [PATCH v3 07/14] checkout: split into switch-branch and restore-files

[Duy Nguyen <pclouds@gmail.com>]

On Tue, Dec 4, 2018 at 6:43 PM Elijah Newren <newren@gmail.com> wrote:
> > > > +--ours::
> > > > +--theirs::
> > > > +       Check out stage #2 ('ours') or #3 ('theirs') for unmerged
> > > > +       paths.
> > > > ++
> > > > +Note that during `git rebase` and `git pull --rebase`, 'ours' and
> > > > +'theirs' may appear swapped; `--ours` gives the version from the
> > > > +branch the changes are rebased onto, while `--theirs` gives the
> > > > +version from the branch that holds your work that is being rebased.
> > > > ++
> > > > +This is because `rebase` is used in a workflow that treats the
> > > > +history at the remote as the shared canonical one, and treats the
> > > > +work done on the branch you are rebasing as the third-party work to
> > > > +be integrated, and you are temporarily assuming the role of the
> > > > +keeper of the canonical history during the rebase.  As the keeper of
> > > > +the canonical history, you need to view the history from the remote
> > > > +as `ours` (i.e. "our shared canonical history"), while what you did
> > > > +on your side branch as `theirs` (i.e. "one contributor's work on top
> > > > +of it").
> > >
> > > Total aside because I'm not sure what you could change here, but man
> > > do I hate this.
> >
> > Uh it's actually documented? I'm always confused by this too. --ours
> > and --theirs at this point are pretty much tied to stage 2 and 3.
> > Nothing I can do about it. But if you could come up with some other
> > option names, then we could make "new ours" to be stage 3 during
> > rebase, for example.
>
> I don't think it's a naming issue, personally.  Years ago we could
> have defined --ours and --theirs differently based on which kind of
> operation we were in the middle of, but you are probably right that
> they are now tied to stage 2 and 3.  But there's another place that we
> might still be able to address this; I think the brain-damage here may
> have just been due to the fact that the recursive merge machinery was
> rather inflexible and required HEAD to be stage 2.  If it were a
> little more flexible, then we might just be able to make this problem
> go away.  Maybe it can still be fixed (I haven't dug too deeply into
> it), but if so, the only fix needed here would be to remove this long
> explanation about why the tool gets things totally backward.

Aha. I' not really deep in this merge business to know if stages 2 and
3 can be swapped. This is right up your alley. I'll just leave it to
you.

> > > > +'git switch-branch' -c|-C <new_branch> [<start_point>]::
> > > > +
> > > > +       Specifying `-c` causes a new branch to be created as if
> > > > +       linkgit:git-branch[1] were called and then switched to. In
> > > > +       this case you can use the `--track` or `--no-track` options,
> > > > +       which will be passed to 'git branch'.  As a convenience,
> > > > +       `--track` without `-c` implies branch creation; see the
> > > > +       description of `--track` below.
> > >
> > > Can we get rid of --track/--no-track and just provide a flag (which
> > > takes no arguments) for the user to use?  Problems with --track:
> > >   * it's not even in your synopsis
> > >   * user has to repeat themselves (e.g. 'next' in two places from '-c
> > > next --track origin/next'); this repetition is BOTH laborious AND
> > > error-prone
> > >   * it's rather inconsistent: --track is the default due to
> > > auto-vivify when the user specifies nothing but a branch name that
> > > doesn't exist yet, but when the user realizes the branch doesn't exist
> > > yet and asks to have it created then suddenly tracking is not the
> > > default??
> >
> > I don't think --track is default anymore (maybe I haven't updated the
> > man page correctly). The dwim behavior is only activated in
> > switch-branch when you specify --guess to reduce the amount of magic
> > we throw at the user. With that in mind, do we still hide
> > --track/--no-track from switch-branch?
>
> Ooh, you're adding --guess?  Cool, that addresses my concerns, just in
> a different manner.

No it's always there even in git-checkout, just hidden.

> Personally, I'd leave --track/--no-track out.  It's extra mental
> overhead, git branch has options for setting those if they need some
> special non-default setup, and if there is enough demand for it we can
> add it later.  Removing options once published is much harder.

Slightly less convenient (you would need a combination of git-branch
and git-switch-branch, if you avoid git-checkout). But since I don't
think I have ever used this option, I'm fine with leaving it out and
considering adding it back later.

> > > I'm not sure what's best, but here's some food for thought:
> > >
> > >
> > >    git switch-branch <branch>
> > > switches to <branch>, if it exists.  Error cases:
> > >   * If <branch> isn't actually a branch but a <tag> or
> > > <remote-tracking-branch> or <revision>, error out and suggest using
> > > --detach.
> > >   * If <branch> isn't actually a branch but there is a similarly named
> > > <remote-tracking-branch> (e.g. origin/<branch>), then suggest using
> > > -c.
> >
> > I would make these advice so I can hide them. Or if I manage to make
> > all these hints one line then I'll make it unconditional.
> >
> > >   git switch-branch -c <branch>
> > > creates <branch> and, if a relevant-remote-tracking branch exists,
> > > base the branch on that revision and set the new branch up to track
> >
> > Hmm.. this is a bit magical and could be surprising. If I create (and
> > switch to) a new branch foo, I don't necessarily mean tracking
> > origin/foo (I may not even think about origin/foo when I type the
> > command). So tentatively no.
>
> Yeah, if you're adding --guess then I'm happy.  I do think, though,
> that if the user runs switch-branch to a branch that doesn't exist, we
> should check if there is an associated remote-tracking branch so that
> we can provide a better error message and help users learn about
> --guess.  (Also, will there be a short -g form?)

Yeah better error and suggestion is a good idea. And yes the short
form -g is already added (I did try to use it and find --guess too
time consuming even with bash completion support).

> > > > +-f::
> > > > +--force::
> > > > +       Proceed even if the index or the working tree differs from
> > > > +       HEAD.  This is used to throw away local changes.
> > >
> > > Haven't thought through this thoroughly, but do we really need an
> > > option for that instead of telling users to 'git reset --hard HEAD'
> > > before switching branches if they want their stuff thrown away?
> >
> > For me it's just a bit more convenient. Hit an error when switching
> > branch? Recall the command from bash history, stick -f in it and run.
> > Elsewhere I think both Junio and Thomas (or maybe only Junio) suggests
> > moving the "git reset" functionality without moving HEAD to one of
> > these commands, which goes the opposite direction...
>
> Fair enough.

I'm actually still not sure how to move it here (I guess 'here' is
restore-files since we won't move HEAD). All the --mixed, --merge and
--hard are confusing. But maybe we could just make 'git restore-files
--from HEAD -f :/" behave just like "git reset --hard HEAD" (but with
some safety net) But we can leave it for discussion in the next round.

> > > > +--orphan <new_branch>::
> > > > +       Create a new 'orphan' branch, named <new_branch>, started from
> > > > +       <start_point> and switch to it.  The first commit made on this
> > >
> > > What??  started from <start_point>?  The whole point of --orphan is
> > > you have no parent, i.e. no start point.  Also, why does the
> > > explanation reference an argument that wasn't in the immediately
> > > preceding synopsis?
> >
> > I guess bad phrasing. It should be "switch to <start_point> first,
> > then prepare the worktree so that the first commit will have no
> > parent". Or something along that line.
> >
> > You should really review git-checkout.txt btw ;-)
>
> I did after writing several of these comments, and yeah, it really
> needs a clean up.  Seems like something someone would do when writing
> a (partial) replacement or simplified alternative.  ;-)

Heh ;-) Fine I'll do it. I have to read and re-read git-checkout.txt
anyway and already queued up a couple small fixes.

> > > > +       new branch will have no parents and it will be the root of a new
> > > > +       history totally disconnected from all the other branches and
> > > > +       commits.
> > > > ++
> > > > +The index and the working tree are adjusted as if you had previously run
> > > > +"git checkout <start_point>".  This allows you to start a new history
> > > > +that records a set of paths similar to <start_point> by easily running
> > > > +"git commit -a" to make the root commit.
> > > > ++
> > > > +This can be useful when you want to publish the tree from a commit
> > > > +without exposing its full history. You might want to do this to publish
> > > > +an open source branch of a project whose current tree is "clean", but
> > > > +whose full history contains proprietary or otherwise encumbered bits of
> > > > +code.
> > > > ++
> > > > +If you want to start a disconnected history that records a set of paths
> > > > +that is totally different from the one of <start_point>, then you should
> > > > +clear the index and the working tree right after creating the orphan
> > > > +branch by running "git rm -rf ." from the top level of the working tree.
> > > > +Afterwards you will be ready to prepare your new files, repopulating the
> > > > +working tree, by copying them from elsewhere, extracting a tarball, etc.
> > >
> > > Ick.  Seems overly complex.  I'd rather that --orphan defaulted to
> > > clearing the index and working tree, and that one would need to pass
> > > HEAD for <start_point> if you wanted to start out with all those other
> > > files.  That would certainly make the explanation a little clearer to
> > > users, and more natural when they start experimenting with it.
> > >
> > > However, --orphan is pretty special case.  Do we perhaps want to leave
> > > it out of this new command and only include it in checkout?
> >
> > I started this by simply splitting git-checkout in two commands that,
> > combined, can do everything git-checkout can. Then suggestions to have
> > better default came in and I think we started to drift further to
> > _removing_ options and falling back to git-checkout.
> >
> > I think we could still keep "complicated" options as long as they are
> > clearly described and don't surprise users until they figure them out.
> > That way I don't have to go back to git-checkout and deal with all the
> > ambiguation it creates.
>
> Fair enough...though I think it may make sense to also review the
> complicated options and determine if they are overly complicated.  I
> think --orphan qualifies (I stumbled with it a bit for years the
> occasional time I needed to use it), and my small suggestion above
> would simplify both it and its description.  We should probably also
> consider just removing <start_point> as an acceptable argument to
> --orphan; if people want files from some revision after creating an
> orphan branch that's a simple extra command.

It is good that you pointed it out though. I still have to digest this
option before I make more comments, but generally if there's a simpler
(even if new) way to achieve the same thing, I'm all for it.
-- 
Duy