Re: [PATCH/RFC v3 00/14] Introduce new commands switch-branch and restore-files

[Junio C Hamano <gitster@pobox.com>]

Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:

> 'git switch-branch'
>
> - implicit detaching is rejected. If you need to detach, you need to
>   give --detach. Or stick to 'git checkout'.

OK.  Is "auto-vivify the named branch based on a remote-tracking"
also rejected, as it is a confusing behaviour that is a too subtle
and implicit, just like the detaching head is, and require --guess
or sticking to 'git checkout'?  I think it should.

> - -b/-B is renamed to -c/-C with long option names

I did not expect that these two are the only options that would be
out of place with the command name split, but presumably you looked
at all options for both of the two new commands to see if they made
sense in the new context?

> 'git restore-files'
>
> - takes a ref from --from argument, not as a free ref. As a result,
>   '--' is no longer needed. All non-option arguments are pathspec

OK.  That does make things easier to teach, as there is no need for
disambiguation.

> - pathspec is mandatory, you can't do "git restore-files" without any
>   pathspec.
>
> - I just remember -p which is allowed to take no pathspec :( I'll fix
>   it later.

Or leave it out of restore-files as a more advanced feature (just
like detaching with HEAD^0 is left out of switch-branch) that the
user can stick to 'git checkout' to use.

> - Two more fancy features (the "git checkout --index" being the
>   default mode and the backup log for accidental overwrites) are of
>   course still missing. But they are coming.

I am unsure about the wisdom of calling it "--index", though.

The "--index" option is "the command can work only on the index, or
only on the working tree files, or on both the index and the working
tree files, and this option tells it to work in the 'both the index
and the working tree files' mode", but when "restore-files" touches
paths, it always modifies both the index and the working tree, so
the "--index" option does not capture the differences well in this
context [*1*].  As I saw this was described as "not using the usual
'overlay' semantics [*2*]", perhaps --overlay/--no-overlay option
that defaults to --no-overlay is easier to explain.

    side note 1.  I think the original mention of "--index" came in
    the context of contrasting "git reset" with "git checkout".
    "git reset (--hard|--mixed) -- <pathspec>" (that does not move
    HEAD), which does not but perhaps should exist, is very much
    like "git checkout -- <pathspec>", and if "reset" were written
    after the "--index/--cached" convention was established, "reset
    --hard" would have called "reset --index" while "reset --mixed"
    would have been "reset --cached" (i.e. only work on the index
    and not on the working tree).  And "reset --index <directory>"
    would have worked by removing paths in <directory> that are not
    in the HEAD and updating paths in <directory> that are in the
    HEAD, i.e. identical to the non overlay behaviour proposed for
    the "git checkout" command.  So calling the non overlay mode
    "--index" makes sense in the context of discussing "git reset",
    and not in the context of "git checkout".

    side note 2.  "git checkout <tree-ish> <pathspec>" grabs entries
    from the <tree-ish> that patch <pathspec> and adds them to the
    index and checks them out to the working tree.  If the original
    index has entries that match <pathspec> but do not appear in
    <tree-ish>, they are left in the result.  That is "overlaying
    what was taken from the <tree-ish> on top of what is in the
    index".

Having said all that, I will not be looking at the series until 2.20
final.  And I hope more people do the same to concentrate on helping
us prevent the last minute glitch slipping in the final release.

Thanks.