Re: [PATCH v2 8/9] stash show: teach --include-untracked and --only-untracked

[Denton Liu <liu.denton@gmail.com>]

Hi Junio,

On Tue, Feb 09, 2021 at 11:53:06PM -0800, Junio C Hamano wrote:
> > Do this via something like
> >
> > 	GIT_INDEX_FILE=... git read-tree stash stash^3
> >
> > and diffing the resulting tree object against the stash base.
> 
> That explains the implementation, but does not make it clear what
> the implementation wants to achieve.  So we read the tree from stash
> (i.e. working tree) into a temporary index, and then overlay the
> tree of stash^3 (i.e. untracked) on top---which means the resulting
> "index" has the state of the working tree plus the untracked cruft
> in it.  And comparing that with "stash base" (by the way is that a
> term well understood?  I borrowed it for the above review comment,
> which shows that there certainly is need for such a term) would show

I'm not sure if it's a well-understood term but I can't think of any
other meanings for the term so it doesn't seem very ambiguous.

> the diff between the "HEAD" and the state that would have result if
> you were to do an "git add ." in the working tree.  OK.
> 
> > One improvement that this could use for the future is performing the
> > action without writing anything to disk as one would expect this to be a
> > read-only operation. This can be fixed in the future, however.
> 
> Is it so difficult that we have to delay the fix for "the future"?
> After reading two trees into an in-core index, without writing it
> out to any file, all that remains to be done is just a matter of
> running diff-lib.c::do_diff_cache(), no?  I must be missing something.q

No, I don't think it's difficult. It's just my inexperience with this
area of the code.

> > Another limitation of this is that it would be possible to manually
> > craft a stash entry where duplicate untracked files in the stash entry
> > will mask tracked files. This seems like an instance of "Doctor, it
> > hurts when I do this! So don't do that!" so this can be written off.
> 
> Well, when you read the second tree into the in-core index to
> overlay what you read from the working tree state, you can certainly
> report the collision and error it out.

I'll send out my revised patch later today and I was unable to figure
out an easy way of doing this.

Thanks,
Denton