Re: [PATCH] diff: add --ignore-blank-lines option

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Antoine Pelisse <apelisse@xxxxxxxxx> writes:

> So here is a more thorough description of the option:

> - real changes are interesting

OK, I think I can understand it.

> - blank lines that are close enough (less than context size) to
>   interesting changes are considered interesting (recursive definition)

OK.

> - "context" lines are used around each hunk of interesting changes

OK.

> - If two hunks are separated by less than "inter-hunk-context", they
>   will be merged into one.

Makes sense.

> The current implementation does the "interesting changes selection" in a
> single pass.

"current" meaning "the code after this patch is applied"?  Is there
a possible future enhancement hinted here?

> +xdchange_t *xdl_get_hunk(xdchange_t **xscr, xdemitconf_t const *xecfg)
> +{
> +	xdchange_t *xch, *xchp, *lxch;
>  	long max_common = 2 * xecfg->ctxlen + xecfg->interhunkctxlen;
> +	long max_ignorable = xecfg->ctxlen;
> +	unsigned long changes = ULONG_MAX;
> +
> +	/* remove ignorable changes that are too far before other changes */
> +	for (xchp = *xscr; xchp && xchp->ignore; xchp = xchp->next) {
> +		xch = xchp->next;
> +
> +		if (xch == NULL ||
> +		    xch->i1 - (xchp->i1 + xchp->chg1) >= max_ignorable)
> +			*xscr = xch;
> +	}

This strips leading ignorable ones away until we see an unignorable
one.  Looks sane.

> +	if (*xscr == NULL)
> +		return NULL;
> +
> +	lxch = *xscr;

"lxch" remembers the last one that is "interesting".

> +	for (xchp = *xscr, xch = xchp->next; xch; xchp = xch, xch = xch->next) {
> +		long distance = xch->i1 - (xchp->i1 + xchp->chg1);
> +		if (distance > max_common)
>  			break;

If we see large-enough gap, the one we processed last (in xchp) is
the end of the current hunk.  Looks sane.

> +		if (distance < max_ignorable &&
> +		    (!xch->ignore || changes == ULONG_MAX)) {
> +			lxch = xch;
> +			changes = ULONG_MAX;

The current one is made into the "last interesting one we have seen"
and the hunk continues, if either (1) the current one is interesting
by itself, or (2) the last one we saw does not match some
unexplainable criteria to cause changes set to not ULONG_MAX.

Puzzling.

> +		} else if (changes != ULONG_MAX &&
> +			   xch->i1 + changes - (lxch->i1 + lxch->chg1) > max_common) {
> +			break;

If the last one we saw does not match some unexplainable criteria to
cause changes set to not ULONG_MAX, and the distance between this
one and the last "intersting" one is further than the context, this
one will not be a part of the current hunk.

Puzzling.

Could you add comment to the "changes" variable and explain what the
variable means?

> +		} else if (!xch->ignore) {
> +			lxch = xch;
> +			changes = ULONG_MAX;

When this change by itself is interesting, it becomes the "last
interesting one" and the hunk continues.

> +		} else {
> +			if (changes == ULONG_MAX)
> +				changes = 0;
> +			changes += xch->chg2;

Puzzled beyond guessing.  Also it is curious why here and only here
we look at chg2 side of the things, not i1/chg1 in this whole thing.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]