Re: Feedback on git-restore

["Ævar Arnfjörð Bjarmason" <avarab@gmail.com>]

On Wed, May 15 2019, Duy Nguyen wrote:

> On Wed, May 15, 2019 at 09:38:59AM +0000, Poughon Victor wrote:
>> Hi
>>
>> I came across a description of a new git command currently in
>> development called 'git restore'. Since it's still not out, and the
>> original poster [1] seemed to ask for feedback, I though I'd send
>> some here. Hope that's ok!
>>
>
> Absolutely. And this is even better because other people could also
> comment on.
>
>> Reading the documentation [2] I find it very confusing. In
>> particular when comparing the following two commands:
>>
>> $ git restore --staged file
>> $ git restore --worktree file
>>
>> With the current proposal, the first will restore the index from
>> HEAD, while the second will restore the worktree from the index. In
>> other words, the source for the restore is different in both
>> commands, even though neither specify a source!
>>
>> This means that git-restore really does two different things
>> depending on some other not obvious context. Unfortunately that's
>> typical of the (often criticized) obscure interface of git. To be
>> fair that behavior is documented in [2]. But still, having a
>> variable default value for --source depending on other arguments is
>> very confusing.
>
> I think it depends on whether use actively use the index, or you
> mostly ignore it and always do "git commit -a" and friends.
>
> When you do use the index, the "worktree <-> index <-> HEAD" is the
> three stages that you are aware, in that order, and restoring from the
> "next" stage is expected.
>
> It does feel natural for me that we "restore worktree from the index"
> and "restore index from HEAD". But maybe I'm just too used to the old
> way of thinking? Let's see what other people say.
>
> This is also consistent with other commands, for example "git diff
> --staged/--cached" compares the index and HEAD and "git diff" compares
> worktree and the index. You would need extra effort e.g. "git diff
> HEAD" to compare the worktree and HEAD.
>
> If your workflow ignores the index, which should always match HEAD,
> then different default source is practically gone, since
> index == HEAD.

I haven't had time to follow the whole restore/switch effort in any
detail.

One thing that would be really useful (and maybe it even exists, I just
haven't seen it in the mails) is some abbreviated cheatsheet style doc
of before/after in the UI. Similar to cheatsheets like e.g.:

    https://git.wiki.kernel.org/images-git/7/78/Git-svn-cheatsheet.pdf
    http://unix4admins.blogspot.com/2013/03/unix-commands-comparison-sheet.html

As far as I can tell the best examples are your changes to
s/checkout/[reset|switch]/ in various existing docs, that's great, but
isn't so easy to understand at a glance.

>> So in summary, I'd make two recommendations for this command's UX:
>> 1. Make --source default value always HEAD if unspecified
>> 2. Rename --staged to --index
>
> This --index vs --staged was discussed and --staged is a compromise.
> The problem is --index means something different in existing
> commands. It specifies that you want to target both the index _and_
> worktree. --cached on the other hand only targets the index [1].
>
> It's confusing, yes. But --index/--cached is part of Git and we cannot
> just ignore our baggage and redefine --index to "just index". That
> will create more confusion and inconsistency between commands.
> "--index" is simply not available.
>
> So the compromise is we leave --index/--cached alone and gradually
> move to the --staged/--worktree combo (for other commands as well).
> Eventually I hope people will move to the second pair and mostly
> forget about --index/--cached. And in a very long long time in the
> future, maybe we can deprecate/remove/redefine --index/--cached.

We had some discussion around such UI changes in
https://public-inbox.org/git/87zhtqvm66.fsf@evledraar.gmail.com/

I'm not expecting us to agree any more on that ui.version config point
today than then.

But I do think it would be really useful in such a cheatsheet to have a
third column of "here's what the 2nd column look like if we were writing
git today / weren't worried about backwards compatibility".

It would allow us to at least clearly document what we wanted to do, but
decided not to for backwards compatibility, and perhaps such a
lightweight design doc could even inform future steps about deprecation
and/or "ui.version" config etc.

> [1] https://github.com/git/git/blob/pu/Documentation/gitcli.txt#L179-L199
>
>> Some examples of those:
>>
>> $ git restore --index file # reset the index from HEAD
>> $ git restore --worktree file # reset the worktree from HEAD
>
> I should also note that --worktree is the default, you can just write
>
> $ git restore file
>
> and achieve the same thing. Writing --worktree is only needed when you
> want to make it clear to the reader you're restoring the worktree.
>
>> $ git restore --worktree --source=index file # reset the worktree from the index
>> $ git restore --index --worktree file # reset both the index and worktree from HEAD
>> $ git restore file # reset the worktree from HEAD
>>
>> [1] https://news.ycombinator.com/item?id=19907960
>> [2] https://github.com/git/git/blob/pu/Documentation/git-restore.txt
>>
>> Best,
>> Victor
>>
>>
>>