git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Duy Nguyen <pclouds@gmail.com>
To: Poughon Victor <Victor.Poughon@cnes.fr>
Cc: "git@vger.kernel.org" <git@vger.kernel.org>
Subject: Re: Feedback on git-restore
Date: Wed, 15 May 2019 17:30:31 +0700	[thread overview]
Message-ID: <20190515103031.GA29149@ash> (raw)
In-Reply-To: <3E55146A6A81B44A9CB69CAB65908CEA6B91353C@TW-MBX-P01.cnesnet.ad.cnes.fr>

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

  reply	other threads:[~2019-05-15 10:30 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-05-15  9:38 Feedback on git-restore Poughon Victor
2019-05-15 10:30 ` Duy Nguyen [this message]
2019-05-15 10:59   ` Ævar Arnfjörð Bjarmason
2019-05-15 11:16     ` Duy Nguyen
2019-05-16  2:18   ` Junio C Hamano
2019-05-16 12:12     ` Philip Oakley
2019-05-16 12:44       ` Duy Nguyen
2019-05-18 14:19         ` Philip Oakley

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190515103031.GA29149@ash \
    --to=pclouds@gmail.com \
    --cc=Victor.Poughon@cnes.fr \
    --cc=git@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).