Re: [RFC PATCH, WAS: "weird diff output?"] Implement better chunk heuristics.

From: Junio C Hamano <gitster@pobox.com>
Date: Apr 15, 2016, 02:09 UTC
Message-ID: <xmqqbn5bei7x.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <20160415000730.26446-1-sbeller@google.com>

Stefan Beller <sbeller@google.com> writes:

Show 7 quoted lines

>  
> +static int starts_with_emptyline(const char *recs)
> +{
> +	return recs[0] == '\n'; /* CRLF not covered here */
> +}
> +
> +

That's "is-empty-line", not "starts-with" ;-)

Show 9 quoted lines

> +
> +		/*
> +		 * If a group can be moved back and forth, see if there is an
> +		 * empty line in the moving space. If there is an empty line,
> +		 * make sure the last empty line is the end of the group.
> +		 *
> +		 * As we shifted the group forward as far as possible, we only
> +		 * need to shift it back if at all.
> +		 */

Sounds sensible.

> +		if (has_emptyline) {
> +			while (ixs > 0 && recs[ixs - 1]->ha == recs[ix - 1]->ha &&
> +			       xdl_recmatch(recs[ixs - 1]->ptr, recs[ixs - 1]->size, recs[ix - 1]->ptr, recs[ix - 1]->size, flags) &&
> +			       !starts_with_emptyline(recs[ix - 1]->ptr)) {

You probably want to wrap the "hash compares equal and recmatch does say they are the same" into a helper function (to be automatically inlined by the compiler) to make it more readable here. I think is-empty is a lot cheaper than the recmatch so that should probably be done earlier in the && chain.

Show 6 quoted lines

> +				rchg[--ixs] = 1;
> +				rchg[--ix] = 0;
> +
> +				/*
> +				 * This change did not join two change groups,
> +				 * as we did that before already, so there is no

Sorry, cannot quite parse the part before "already".

Show 10 quoted lines

> +				 * need to adapt the other-file, i.e.
> +				 * running
> +				 *     for (; rchg[ixs - 1]; ixs--);
> +				 *     while (rchgo[--ixo]);
> +				 */
> +			}
> +		}
>  	}
>  
>  	return 0;