Re: [PATCH v2 00/16] Add new command 'restore'

From: Junio C Hamano <gitster@pobox.com>
Date: Apr 12, 2019, 05:14 UTC
Message-ID: <xmqq4l73ztxf.fsf@gitster-ct.c.googlers.com>
In-Reply-To: <20190411131218.19195-1-pclouds@gmail.com>

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

Show 6 quoted lines

> v2 should address all the comments from v1. I still haven't done that
> --intent-to-add thing yet even though I'd like to make it default
> behavior too. Anyway changes are
>
> - --index is renamed to --staged to avoid conflict with --index from
>   git-apply, which has different meaning.

I think this is a reasonable compromise. Documentation/gitcli.txt needs to be updated for this new addition, where it currently only talks about the distinction between "--cached" and "--index".

> - git-rm learns about --staged as an alias of --cached (in fact it's
>   more the other way around). This is to keep suggestions consistent
>   because we tell people to do "git foo --staged" everywhere.

I am not sure 100% about this one. If "--staged" is only about updating the index without touching the working tree, then existing "--cached" is already serving its purpose and there is no need to add yet another. Why did we need it for "restore-paths" in the first place? Is it because the target of restoring can be one of the three (index+working-tree, index-only, or working-tree only), not just the traditional two (index+working-tree that is signalled by "--index", and index-only that is signalled by "--cached")?

I think it would make sense to introduce --staged to "git rm", if we teach the third target (i.e. "working-tree only") to the command, but otherwise, I am not sure if it would help reducing conflusion.