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@xxxxxxxxxxxxxxxxxxx/ 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 >> >> >>