Feedback on git-restore

Feedback on git-restore

[Poughon Victor]

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!

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.

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

Some examples of those:

$ git restore --index file # reset the index from HEAD
$ git restore --worktree file # reset the worktree from HEAD
$ 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

Re: Feedback on git-restore

[Duy Nguyen]

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.

> 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.

[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
> 
> 
> 

Re: Feedback on git-restore

[Ævar Arnfjörð Bjarmason]

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
>>
>>
>>

Re: Feedback on git-restore

[Duy Nguyen]

On Wed, May 15, 2019 at 12:59:17PM +0200, Ævar Arnfjörð Bjarmason wrote:
> 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.

OK the cheatsheet would be like this (I'm dropping "git" prefix to
keep it short)

checkout <branch>          -> switch <branch> (same "dwim" behavior)
checkout <revision>        -> switch --detach <revision>
checkout -b <branch> <rev> -> switch -c <branch> <rev>
checkout <path>            -> restore [--worktree] <path>
checkout <rev> -- <path>   -> restore --source=<rev> --worktree --staged <path>
reset <path>               -> restore --staged <path>
reset <rev> -- <path>      -> restore --source=<rev> --staged <path>
reset --hard               -> restore --source=HEAD --staged --worktree :/
(not available now)        -> restore --source=<rev> [--worktree] <path>

There are other behavior changes to improve things, e.g. --no-overlay
is the default in "git restore". But those can't be shown in a cheatsheet.
--
Duy

Re: Feedback on git-restore

[Junio C Hamano]

Duy Nguyen <pclouds@gmail.com> writes:

> 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.

I am somewhat sympathetic to unexperienced users who do not "get"
Git here, but because I still think you cannot make effective use of
Git without understanding the workflow using the index as an "extra"
level sitting between the working tree and the commits, I think it
is a learning experience worth investing in for a new user to get
used to the way of thinking you showed in the above paragraphs.

> 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.

Exactly.

> 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".

Another thing worth pointing out here may be that a user would stop
feeling it a baggage once the index-centric way of thinking sinks
in.  Because the index is so central to the local use of Git
workflow (i.e. "I am just cloning and pulling from the outside world
to fllow along" does not count), "just the working tree and not the
index" mode is an anomaly, until/unless you start talking about
going backwards (e.g. "I've smudged my working tree by mistake, and
need to recover by copying something out to it", which is the mental
model of "restore").

	Side note: while at the mechanical level what it does is an
	equivalent to what "checkout -- <paths>" does, the mental
	models are different.  "checkout" is still a way to move
	forward "I need the contents of these paths in my next
	commit to look like those in that tree-ish, so copy them out
	to the working tree (to e.g. compile test) and also to the
	index (for e.g. the ease of committing after testing is
	done)".

> So the compromise is we leave --index/--cached alone and gradually
> move to the --staged/--worktree combo (for other commands as well).

I think what is truly new and valuable is the "working tree only,
bypassing the index" mode, but I am not sure if we want to always
force us to say both when we want to affect both the index and the
working tree, as the index is still central to the local workflow.
I am not sure if there is a wide agreement with such a "gradually
move to" plan.

Also, when you want to only affect the indexed contents, wouldn't
existing "--cached" suffice?

IOW, I do not mind --staged as a synonym for --cached, and --worktree
as a useful addition, but I do not think these new pair should replace
the existing ones.

Re: Feedback on git-restore

[Philip Oakley]

On 16/05/2019 03:18, Junio C Hamano wrote:
> Duy Nguyen <pclouds@gmail.com> writes:
>
>> 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.
> I am somewhat sympathetic to unexperienced users who do not "get"
> Git here, but because I still think you cannot make effective use of
> Git without understanding the workflow using the index as an "extra"
> level sitting between the working tree and the commits, I think it
> is a learning experience worth investing in for a new user to get
> used to the way of thinking you showed in the above paragraphs.
I would agree.

"the Index" is grossly under-documented and explained. I still don't 
have a proper understanding of all the multiple Nuances regarding the 
Index concept. Only a moment ago I was reading a thread that talked 
about having multiple indexes and the wrong one being referenced or locked.

I tend to map the Index to an old-fashioned 'out-box' on the corner of 
one's desk (from the days when everything was paper and cc really meant 
a carbon copy made with carbon paper). One would work on a document and 
when 'finished' place it in the out-box, which also acted as a 
work-in-progress (wip) box. Only later would a secretary/mail-person 
come around to see what needed filing/sending.

The fact that there can be many indexes, with multiple stages (all 
un-named) is a further source of confusion. For the average user, the 
only visible artefacts are those in the file system, and perhaps the 
crisp memory of the last commit. So I can see where Victor is coming 
from. I don't think we should be solely relying on the experience of 
user mistakes for their learning.

Maybe we need a `git index` command to make it far more visible to 
average users (or `git staging-area --show`, with a --cached option ;-).
>> 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.
> Exactly.
>
>> 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".
> Another thing worth pointing out here may be that a user would stop
> feeling it a baggage once the index-centric way of thinking sinks
> in.  Because the index is so central to the local use of Git
> workflow (i.e. "I am just cloning and pulling from the outside world
> to fllow along" does not count), "just the working tree and not the
> index" mode is an anomaly, until/unless you start talking about
> going backwards (e.g. "I've smudged my working tree by mistake, and
> need to recover by copying something out to it", which is the mental
> model of "restore").
>
> 	Side note: while at the mechanical level what it does is an
> 	equivalent to what "checkout -- <paths>" does, the mental
> 	models are different.  "checkout" is still a way to move
> 	forward "I need the contents of these paths in my next
> 	commit to look like those in that tree-ish, so copy them out
> 	to the working tree (to e.g. compile test) and also to the
> 	index (for e.g. the ease of committing after testing is
> 	done)".
>
>> So the compromise is we leave --index/--cached alone and gradually
>> move to the --staged/--worktree combo (for other commands as well).
> I think what is truly new and valuable is the "working tree only,
> bypassing the index" mode, but I am not sure if we want to always
> force us to say both when we want to affect both the index and the
> working tree, as the index is still central to the local workflow.
> I am not sure if there is a wide agreement with such a "gradually
> move to" plan.
>
> Also, when you want to only affect the indexed contents, wouldn't
> existing "--cached" suffice?
>
> IOW, I do not mind --staged as a synonym for --cached, and --worktree
> as a useful addition, but I do not think these new pair should replace
> the existing ones.
If the Index is central it need to be to get out from behind the 
curtain. Just sayin'.

--
Philip

Re: Feedback on git-restore

[Duy Nguyen]

On Thu, May 16, 2019 at 7:12 PM Philip Oakley <philipoakley@iee.org> wrote:
> Maybe we need a `git index` command to make it far more visible to
> average users (or `git staging-area --show`, with a --cached option ;-).

Not commenting on the other parts (and also Junio's mail) since I
still need more time to process.

But how about we see the index as a "commit-in-progress" (or "staging
area" which is almost the same, maybe "commit area" is better)?

You can't make a commit visible unless you check it out (and then can
use various tools available to work on filesystem), or you use git
diff/show to examine it. The index is treated pretty much the same
way, except that it does not have a proper SHA-1 yet because it's
still a work in progress.

Short of creating a fuse filesystem to show you the index content (as
read-only files) I don't see any better way that you can actually see
the index without checking it out.

PS. Yes I ignored the role of the index during a merge conflict. Not
relying on the index for conflict handling might be possible, but I'm
not going to touch that topic, way out of my area.
-- 
Duy

Re: Feedback on git-restore

[Philip Oakley]

Hi Duy,

On 16/05/2019 13:44, Duy Nguyen wrote:
> On Thu, May 16, 2019 at 7:12 PM Philip Oakley <philipoakley@iee.org> wrote:
>> Maybe we need a `git index` command to make it far more visible to
>> average users (or `git staging-area --show`, with a --cached option ;-).
> Not commenting on the other parts (and also Junio's mail) since I
> still need more time to process.
>
> But how about we see the index as a "commit-in-progress" (or "staging
> area" which is almost the same, maybe "commit area" is better)?
I'd agree.

My initial comment was more about the cognitive disconnect between 
Victor's original reply where he has the commit and the file system as 
the (only) firm reference points, and then your reply, which makes the 
case that the _Index_ is central.

It was that disconnect that I wanted to highlight, which is at a level 
above the discussion on just the restore command.

There was a lot of discussion a year or three back about the terminology 
of cache/Index/staging for this middle area. It probably wasn't resolved 
fully as it only discussed the name, rather than why the concept wasn't 
well understood, and what was needed to fix _that_.
>
> You can't make a commit visible unless you check it out (and then can
> use various tools available to work on filesystem), or you use git
> diff/show to examine it. The index is treated pretty much the same
> way, except that it does not have a proper SHA-1 yet because it's
> still a work in progress.
I'd say most users feel happy about the fixedness of their last commit, 
and probably feel happy with the ability to view it via various tools 
(especially if pushed to their GitHub/Lab/Bucket repo). The apparently 
transient nature of the index is part of its Achilles heel. We talk too 
easily about 'blowing it away' [1].  Such phrases reinforce the user 
belief that they should treat it as a bit of a swamp area.

We don't have an index_log of the changes to the staging tree, as files 
are added or removed. That view implies that the index is simply the 
current work-in-progress (wip) tree, and doesn't contain anything else..
>
> Short of creating a fuse filesystem to show you the index content (as
> read-only files) I don't see any better way that you can actually see
> the index without checking it out.
Surely? the 'git status' command [1] is, in a way, trying to be that 
view, but we just haven't told the users
>
> PS. Yes I ignored the role of the index during a merge conflict. Not
> relying on the index for conflict handling might be possible, but I'm
> not going to touch that topic, way out of my area.
The role during merges would be part of that missing conceptual 
structure. Our documentation is doing a little too much of the 'ignore 
what's behind the curtain' [2]. I've certainly fallen for that often. 
The need for various web explanations e.g. [3,4] also suggest we could 
be more assertive in our top level user documentation about 'index is 
one of the most important data structures in git'.

In summary, I think the problem that Victor alludes to is at a higher 
level with respect to how we document what the staging area / index is 
and does.

Philip

[1] e.g. https://shafiul.github.io//gitbook/1_the_git_index.html
[2] Wizard of Oz, Powerful concept, vs loose tools.
[3] 
https://stackoverflow.com/questions/4084921/what-does-the-git-index-contain-exactly,
[4] https://mincong-h.github.io/2018/04/28/git-index/