Re: [PATCH 02/11] tree-diff: consolidate code for emitting diffs and recursion in one place

[Junio C Hamano <gitster@pobox.com>]

Kirill Smelkov <kirr@mns.spb.ru> writes:

> +static void show_path(struct strbuf *base, struct diff_options *opt,
> +		      struct tree_desc *t1, struct tree_desc *t2)
>  {
>  	unsigned mode;
>  	const char *path;
> -	const unsigned char *sha1 = tree_entry_extract(desc, &path, &mode);
> -	int pathlen = tree_entry_len(&desc->entry);
> +	const unsigned char *sha1;
> +	int pathlen;
>  	int old_baselen = base->len;
> +	int isdir, recurse = 0, emitthis = 1;
> +
> +	/* at least something has to be valid */
> +	assert(t1 || t2);
> +
> +	if (t2) {
> +		/* path present in resulting tree */
> +		sha1 = tree_entry_extract(t2, &path, &mode);
> +		pathlen = tree_entry_len(&t2->entry);
> +		isdir = S_ISDIR(mode);
> +	}
> +	else {
> +		/* a path was removed - take path from parent. Also take
> +		 * mode from parent, to decide on recursion.
> +		 */
> +		tree_entry_extract(t1, &path, &mode);
> +		pathlen = tree_entry_len(&t1->entry);
> +
> +		isdir = S_ISDIR(mode);
> +		sha1 = NULL;
> +		mode = 0;
> +	}
> +
> +	if (DIFF_OPT_TST(opt, RECURSIVE) && isdir) {
> +		recurse = 1;
> +		emitthis = DIFF_OPT_TST(opt, TREE_IN_RECURSIVE);
> +	}
>  
>  	strbuf_add(base, path, pathlen);
> -	if (DIFF_OPT_TST(opt, RECURSIVE) && S_ISDIR(mode)) {
> -		if (DIFF_OPT_TST(opt, TREE_IN_RECURSIVE))
> -			opt->add_remove(opt, *prefix, mode, sha1, 1, base->buf, 0);
>  
> +	if (emitthis)
> +		emit_diff(opt, base, t1, t2);
> +
> +	if (recurse) {
>  		strbuf_addch(base, '/');
> -		diff_tree_sha1(*prefix == '-' ? sha1 : NULL,
> -			       *prefix == '+' ? sha1 : NULL, base->buf, opt);
> -	} else
> -		opt->add_remove(opt, prefix[0], mode, sha1, 1, base->buf, 0);
> +		diff_tree_sha1(t1 ? t1->entry.sha1 : NULL,
> +			       t2 ? t2->entry.sha1 : NULL, base->buf, opt);
> +	}


After this step, "sha1" is assigned but never gets used.  Please
double-check the fix-up I queued in the series before merging it to
'pu'.

Thanks.