Denton Liu <liu.denton@xxxxxxxxx> writes: > Hi Junio, > > On Tue, Feb 16, 2021 at 12:22:52PM -0800, Junio C Hamano wrote: >> builtin/stash.c | 3 ++- >> 1 file changed, 2 insertions(+), 1 deletion(-) >> >> diff --git c/builtin/stash.c w/builtin/stash.c >> index c788a3e236..7e0204bd8a 100644 >> --- c/builtin/stash.c >> +++ w/builtin/stash.c >> @@ -807,10 +807,11 @@ static void diff_include_untracked(const struct stash_info *info, struct diff_op >> init_tree_desc(&tree_desc[i], tree[i]->buffer, tree[i]->size); >> } >> >> + /* mimic "git read-tree W U" without "-m" */ >> unpack_tree_opt.head_idx = -1; >> unpack_tree_opt.src_index = &the_index; >> unpack_tree_opt.dst_index = &the_index; >> - unpack_tree_opt.fn = twoway_merge; >> + unpack_tree_opt.fn = NULL; > > Perhaps it would be even more clear if we just removed this line > entirely, otherwise it may give future readers a false impression that > .fn is significant in any way. Assignment of something concrete like "twoway_merge" to .fn when it is a no-op was misleading, but between NULL or uninitialized, both state clear that it is not used, so I am OK with either. > Aside from that, both of your SQUASH??? commits look good to me. Thanks > for tying up the loose ends. We may want to write a custom unpack_trees() callback for this, and use it here to catch "this third tree claims to be untracked, but why does it have an entry that overlaps and/or D/F-conflicts with the entry from the working tree side?" that you talked about in the log message, but I think it is better to leave it for future [*], as the feature should already be usable in its current shape. Thanks. [Footnote] * Yes, I didn't say "we can leave it", but said "it is better to leave it"; as long as we do not declare victory too early and end up shipping a half-baked unusable mess, I think it is better to wrap a topic early and plan to make it nicer in a future follow-up. To encourage such future refinements, however, you may add a NEEDSWORK comment in the code as a reminder for our future selves.
