Re: [PATCH 1/4] diffcore-pickaxe: refactor diffcore_pickaxe()

["René Scharfe" <rene.scharfe@lsrfire.ath.cx>]

Junio C Hamano schrieb:
>  void diffcore_pickaxe(const char *needle, int opts)
>  {
>  	struct diff_queue_struct *q = &diff_queued_diff;
> @@ -75,29 +107,14 @@ void diffcore_pickaxe(const char *needle, int opts)
>  		/* Showing the whole changeset if needle exists */
>  		for (i = has_changes = 0; !has_changes && i < q->nr; i++) {
>  			struct diff_filepair *p = q->queue[i];
> -			if (!DIFF_FILE_VALID(p->one)) {
> -				if (!DIFF_FILE_VALID(p->two))
> -					continue; /* ignore unmerged */
> -				/* created */
> -				if (contains(p->two, needle, len, regexp))
> -					has_changes++;
> -			}
> -			else if (!DIFF_FILE_VALID(p->two)) {
> -				if (contains(p->one, needle, len, regexp))
> -					has_changes++;
> -			}
> -			else if (!diff_unmodified_pair(p) &&
> -				 contains(p->one, needle, len, regexp) !=
> -				 contains(p->two, needle, len, regexp))
> -				has_changes++;
> +			if (pickaxe_match_one(p, needle, len, regexp))
> +				return; /* not munge the queue */
>  		}
> -		if (has_changes)
> -			return; /* not munge the queue */
>  
> -		/* otherwise we will clear the whole queue
> -		 * by copying the empty outq at the end of this
> -		 * function, but first clear the current entries
> -		 * in the queue.
> +		/*
> +		 * otherwise we will clear the whole queue by copying
> +		 * the empty outq at the end of this function, but
> +		 * first clear the current entries in the queue.
>  		 */
>  		for (i = 0; i < q->nr; i++)
>  			diff_free_filepair(q->queue[i]);
> @@ -106,24 +123,8 @@ void diffcore_pickaxe(const char *needle, int opts)
>  		/* Showing only the filepairs that has the needle */
>  		for (i = 0; i < q->nr; i++) {
>  			struct diff_filepair *p = q->queue[i];
> -			has_changes = 0;
> -			if (!DIFF_FILE_VALID(p->one)) {
> -				if (!DIFF_FILE_VALID(p->two))
> -					; /* ignore unmerged */
> -				/* created */
> -				else if (contains(p->two, needle, len, regexp))
> -					has_changes = 1;
> -			}
> -			else if (!DIFF_FILE_VALID(p->two)) {
> -				if (contains(p->one, needle, len, regexp))
> -					has_changes = 1;
> -			}
> -			else if (!diff_unmodified_pair(p) &&
> -				 contains(p->one, needle, len, regexp) !=
> -				 contains(p->two, needle, len, regexp))
> -				has_changes = 1;
>  
> -			if (has_changes)
> +			if (pickaxe_match_one(p, needle, len, regexp))
>  				diff_q(&outq, p);
>  			else
>  				diff_free_filepair(p);

You might want to squash this in, because has_changes is now constantly set
to zero:

diff --git a/diffcore-pickaxe.c b/diffcore-pickaxe.c
index d27e725..d294d29 100644
--- a/diffcore-pickaxe.c
+++ b/diffcore-pickaxe.c
@@ -84,7 +84,7 @@ void diffcore_pickaxe(const char *needle, int opts)
 {
 	struct diff_queue_struct *q = &diff_queued_diff;
 	unsigned long len = strlen(needle);
-	int i, has_changes;
+	int i;
 	regex_t regex, *regexp = NULL;
 	struct diff_queue_struct outq;
 	outq.queue = NULL;
@@ -105,7 +105,7 @@ void diffcore_pickaxe(const char *needle, int opts)
 
 	if (opts & DIFF_PICKAXE_ALL) {
 		/* Showing the whole changeset if needle exists */
-		for (i = has_changes = 0; !has_changes && i < q->nr; i++) {
+		for (i = 0; i < q->nr; i++) {
 			struct diff_filepair *p = q->queue[i];
 			if (pickaxe_match_one(p, needle, len, regexp))
 				return; /* not munge the queue */