Re: diff: --quiet does not imply --exit-code if --diff-filter is present

2 messages, 2 authors, 2016-06-15 · open the first message on its own page

Re: diff: --quiet does not imply --exit-code if --diff-filter is present

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:51:21

Jeff King [off-list ref] writes:

Commit 2cfe8a6 (diff --quiet: disable optimization when
--diff-filter=X is used, 2011-03-16) fixes this in some
cases by disabling the optimization when a filter is
present. However, it only tweaked run_diff_files, missing
the similar case in diff_tree. Thus the fix worked only for
diffing the working tree and index, but not between trees.

Thanks; a natural question is if we need the same for diff-index, then.

quoted hunk

diff --git a/tree-diff.c b/tree-diff.c
index 7a79660..3d08f78 100644
--- a/tree-diff.c
+++ b/tree-diff.c

@@ -143,6 +143,7 @@ int diff_tree(struct tree_desc *t1, struct tree_desc *t2,
 
 	for (;;) {
 		if (DIFF_OPT_TST(opt, QUICK) &&
+		    !opt->filter &&
 		    DIFF_OPT_TST(opt, HAS_CHANGES))
 			break;
 		if (opt->pathspec.nr) {

We probably want to have a helper in diff.c that does

	int diff_can_quit_early(struct diff_options *opt)
        {
        	return (DIFF_OPT_TST(opt, QUICK) &&
                	!opt->filter &&
                        DIFF_OPT_TST(opt, HAS_CHANGES));
	}

It is possible for us to later add new diffcore transformations that need
a similar "do not stop feeding early, as results may be filtered".

Re: diff: --quiet does not imply --exit-code if --diff-filter is present

From: Jeff King <hidden>
Date: 2016-06-15 22:51:21

On Tue, May 31, 2011 at 08:46:29AM -0700, Junio C Hamano wrote:

Jeff King [off-list ref] writes:

quoted

Commit 2cfe8a6 (diff --quiet: disable optimization when
--diff-filter=X is used, 2011-03-16) fixes this in some
cases by disabling the optimization when a filter is
present. However, it only tweaked run_diff_files, missing
the similar case in diff_tree. Thus the fix worked only for
diffing the working tree and index, but not between trees.

Thanks; a natural question is if we need the same for diff-index, then.

No, it calls straight into unpack_trees. So I think the question is
"should unpack_trees respect the QUICK optimization". I suspect it
didn't happen simply because unpack_trees is so complex, and there are
probably corner cases with merging.

We probably want to have a helper in diff.c that does

	int diff_can_quit_early(struct diff_options *opt)
        {
        	return (DIFF_OPT_TST(opt, QUICK) &&
                	!opt->filter &&
                        DIFF_OPT_TST(opt, HAS_CHANGES));
	}

It is possible for us to later add new diffcore transformations that need
a similar "do not stop feeding early, as results may be filtered".

Yeah, that is a good refactoring. It's more readable, and it would have
prevented this bug. :)

-Peff

`h`	back out one level
`j`	next message in thread
`k`	previous message in thread
`l`	drill in
`Esc`	close help / fold thread tree
`?`	toggle this help