Re: [PATCH v3 1/2] stash show: teach --include-untracked and --only-untracked

From: Junio C Hamano <gitster@pobox.com>
To: Denton Liu <liu.denton@gmail.com>
Cc: Git Mailing List <git@vger.kernel.org>,
	Eric Sunshine <sunshine@sunshineco.com>
Subject: Re: [PATCH v3 1/2] stash show: teach --include-untracked and --only-untracked
Date: Tue, 16 Feb 2021 12:22:52 -0800	[thread overview]
Message-ID: <xmqqczwzpxsz.fsf@gitster.c.googlers.com> (raw)
In-Reply-To: <85b81f2f06bd1b40ee2de220cc84dd74b425daf3.1613459475.git.liu.denton@gmail.com> (Denton Liu's message of "Mon, 15 Feb 2021 23:11:56 -0800")

Denton Liu <liu.denton@gmail.com> writes:

> +static void diff_include_untracked(const struct stash_info *info, struct diff_options *diff_opt)
> +{
> +	const struct object_id *oid[] = { &info->w_commit, &info->u_tree };
> +	struct tree *tree[ARRAY_SIZE(oid)];
> +	struct tree_desc tree_desc[ARRAY_SIZE(oid)];
> +	struct unpack_trees_options unpack_tree_opt = { 0 };
> +	int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(oid); i++) {
> +		tree[i] = parse_tree_indirect(oid[i]);
> +		if (parse_tree(tree[i]) < 0)
> +			die(_("failed to parse tree"));
> +		init_tree_desc(&tree_desc[i], tree[i]->buffer, tree[i]->size);
> +	}
> +
> +	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;

OK, it looks like this was borrowed from read_tree implementation
for reading two trees into the index, sort-of, but is a bit funny.

The setting of .fn to twoway_merge is misleading.  The .fn callback
is to be used when .merge is set (otherwise nothing should call it
inside unpack-trees.c), but nobody seems to set opt.merge to true.

> +	if (unpack_trees(ARRAY_SIZE(tree_desc), tree_desc, &unpack_tree_opt))
> +		die(_("failed to unpack trees"));
> +
> +	do_diff_cache(&info->b_commit, diff_opt);
> +}

Nice to see that it was just a simple matter of programming ;-)



 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;
 
 	if (unpack_trees(ARRAY_SIZE(tree_desc), tree_desc, &unpack_tree_opt))
 		die(_("failed to unpack trees"));
