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

Hi Junio,

On Tue, Feb 09, 2021 at 11:53:06PM -0800, Junio C Hamano wrote:

quoted

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.

quoted

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.

quoted

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