From: Junio C Hamano <gitster@pobox.com>
Date: Thu, 27 Oct 2011 18:36:48 GMT
Subject: Re: [PATCH/WIP 05/11] symbolize return values of tree_entry_interesting()
Message-ID: <7vbot2z3gf.fsf@alter.siamese.dyndns.org>
In-Reply-To: <1319438176-7304-6-git-send-email-pclouds@gmail.com>

Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:

> This helps extending the value later on for "interesting, but cannot
> decide if the entry truely matches yet" (ie. prefix matches)
>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>

Good change; it is a basic code hygiene to avoid magic constants anyway.

> diff --git a/tree-diff.c b/tree-diff.c
> index 6782484..25cc981 100644
> --- a/tree-diff.c
> +++ b/tree-diff.c
> @@ -114,12 +114,13 @@ static void show_entry(struct diff_options *opt, const char *prefix,
>  }
>  
>  static void skip_uninteresting(struct tree_desc *t, struct strbuf *base,
> -			       struct diff_options *opt, int *match)
> +			       struct diff_options *opt,
> +			       enum interesting *match)
>  {
>  	while (t->size) {
>  		*match = tree_entry_interesting(&t->entry, base, 0, &opt->pathspec);
>  		if (*match) {
> -			if (*match < 0)
> +			if (*match == all_entries_not_interesting)
>  				t->size = 0;
>  			break;
>  		}

The caller of this function needs to be updated as well.

But I have to wonder why this skip_uninteresting() does not peek the
original value of *match and skip, which is the loop structure the other
caller of tree_entry_interesting() in this file has.

diff --git a/tree-diff.c b/tree-diff.c
index 25cc981..de4ba28 100644
--- a/tree-diff.c
+++ b/tree-diff.c
@@ -133,7 +133,7 @@ int diff_tree(struct tree_desc *t1, struct tree_desc *t2,
 {
 	struct strbuf base;
 	int baselen = strlen(base_str);
-	int t1_match = 0, t2_match = 0;
+	enum interesting t1_match, t2_match;
 
 	/* Enable recursion indefinitely */
 	opt->pathspec.recursive = DIFF_OPT_TST(opt, RECURSIVE);
@@ -142,6 +142,9 @@ int diff_tree(struct tree_desc *t1, struct tree_desc *t2,
 	strbuf_init(&base, PATH_MAX);
 	strbuf_add(&base, base_str, baselen);
 
+	/* Initialize to something other than all_entries_not_interesting */
+	t1_match = t2_match = entry_not_interesting;
+
 	for (;;) {
 		if (diff_can_quit_early(opt))
 			break;