Re: [PATCH 0/8] introduce no-overlay and cached mode in git checkout

From: Thomas Gummerer <t.gummerer@gmail.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: git@vger.kernel.org, "Nguyễn Thái Ngọc Duy" <pclouds@gmail.com>,
	"Elijah Newren" <newren@gmail.com>
Subject: Re: [PATCH 0/8] introduce no-overlay and cached mode in git checkout
Date: Thu, 20 Dec 2018 08:43:51 +0000	[thread overview]
Message-ID: <20181220084351.GA8083@hank.intra.tgummerer.com> (raw)
In-Reply-To: <xmqqa7le2gc2.fsf@gitster-ct.c.googlers.com>

On 12/10, Junio C Hamano wrote:
> Thomas Gummerer <t.gummerer@gmail.com> writes:
> 
> > Basically the idea is to also delete files when the match <pathspec>
> > in 'git checkout <tree-ish> -- <pathspec>' in the current tree, but
> > don't match <pathspec> in <tree-ish>.
> 
> I cannot quite parse it, but perhaps.
> 
> 	"git checkout --no-overlay <tree-ish> -- <pathspec>" can
> 	remove paths in the index and in the working tree that match
> 	<pathspec>, if they do not appear in <tree-ish>.
> 
> If a new file D/F is in the index and in the working tree but not in
> HEAD, "git checkout HEAD D/" or "git checkout HEAD D/F" would not
> remove D/F from the index or the working tree.
> 
> With the --no-overlay option, it would, and that is often closer to
> the wish of the user who wanted to say "restore the working tree
> state of D/ (or D/F) from the state recorded in HEAD".

Yeah thanks, that reads much better.

> > The final step in the series is to actually make use of this in 'git
> > stash', which simplifies the code there a bit.  I am however happy to
> > hold off on this step until the stash-in-C series is merged, so we
> > don't delay that further.
> 
> I think that is probably a good idea, for now.
> 
> > In addition to the no-overlay mode, we also add a --cached mode, which
> > works only on the index, thus similar to 'git reset <tree-ish> -- <pathspec>'.
> >
> > Actually deprecating 'git reset <tree-ish> -- <pathspec>' should come
> > later,...
> 
> Or we may not even need to deprecate it.  IIRC, what "stash" wished
> to exist was "git reset --hard <tree-ish> -- <pathspec>", which, if
> the command followed "--cached/--index" convention, would have been
> called "git reset --index ...".  Did we actually have the need for
> "--cached" mode?

I don't have a pressing need for "--cached".  I mainly included it
because you described "git reset <tree-ish> -- <pathspec>" as bad UI
in the original thread [*1*], which after reading that message I agree
with.  It also seemed to cause some confusion in [*2*].  Since it was
fairly easy to introduce a "--cached" mode it seemed like a potential
UI improvement in the long term to deprecate 'git reset <tree-ish> --
<pathspec>'.

That said, this series is probably not the right place to introduce
this feature, as it should mainly be focused on the no-overlay mode.
I'll drop the patch from v2.

We can revisit whether we want to introduce a "--cached" mode in 'git
checkout' at some point in the future.  

> > probably not before Duy's restore-files command lands, as 'git
> > checkout --no-overlay <tree-ish> -- <pathspec>' is a bit cumbersome to
> > type compared to 'git reset <tree-ish> -- <pathspec>'.
> 
> Yes, between "checkout --cached" and "checkout --no-overlay", the
> latter is much more important, as the latter is what a missing "git
> reset --hard <tree-ish> -- <pathspec>" would have been, but the
> former can be written with an existing command.
> 
> > My hope is also that the no-overlay mode could become the new default
> > in the restore-files command Duy is currently working on.
> 
> Yup, that is my hope, too ;-).

*1*: <xmqq4loqplou.fsf@gitster.mtv.corp.google.com>
*2*: <alpine.LFD.2.21.1812081103500.29142@localhost.localdomain>