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:

> +xdchange_t *xdl_get_hunk(xdchange_t **xscr, xdemitconf_t const *xecfg) {
>  	xdchange_t *xch, *xchp;
>  	long max_common = 2 * xecfg->ctxlen + xecfg->interhunkctxlen;
> +	long ignorable_context = max_common / 2 - 1;

Could you explain how this math works?  Also the logic to use it
when either the previous or the current one is "blank only" (as
opposed to having two different settings for "both are blank only"
and "only one of them is, and the other is not", for example)?

The normal case when neither is blank only, we refrain from
collapsing two adjacent xdchanges if the end of xchp (i.e. the
previous one) is before the beginning of xch (i.e. the current one
we are looking at) by more than max_common lines, which makes sense
to me because we count one ctxlen for the trailing context for xchp,
interhunkctxlen to force collapsing, and another ctxlen for the
leading context for xch.

When we have 

    - zero or more "blank only" changes, followed by
    - a meaningful change A, followed by
    - zero or more blank-only changes, followed by
    - a meaningful change C,

we may want to have either two hunks (A with context around it, and
C with context around it) or a single hunk (precontext before A, all
the lines from the beginning of A to the end of C, and postcontext
after C).  In either case, we want to discard the leading "blank
only" changes.

I can sort-of see how the leading "blank only" changes are discarded
in the loop (but not quite---you can ignore everything without any
"thresh", until you set "interesting" to true, i.e. seeing A, no?).

It is not clear to me how you are counting the distance between the
end of A and the beginning of C, which I think is all that matters,
to make the decision to coalesce (or not to coalesce) the above into
a single hunk, without looking ahead to find the next xdchange that
is not marked with xch->ignore flag (that is, when looking at A,
find the beginning of C to see if C.begin-A.end is within the usual
max_common).

Confused...

> +	int interesting = 0;
>
> -	for (xchp = xscr, xch = xscr->next; xch; xchp = xch, xch = xch->next)
> -		if (xch->i1 - (xchp->i1 + xchp->chg1) > max_common)
> -			break;
> +	for (xchp = *xscr, xch = (*xscr)->next; xch; xchp = xch, xch = xch->next) {
> +		long thresh;
> +		if (xchp->ignore || xch->ignore)
> +			thresh = ignorable_context;
> +		else
> +			thresh = max_common;
> +
> +		if (!xchp->ignore)
> +			interesting = 1;
> +
> +		if (xch->i1 - (xchp->i1 + xchp->chg1) > thresh) {
> +			if (interesting)
> +				break;
> +			else
> +				*xscr = xch;
> +		}
> +	}
> +
> +	if (!interesting && xchp->ignore)
> +		*xscr = NULL;
>
>  	return xchp;
>  }
> @@ -139,7 +159,9 @@ int xdl_emit_diff(xdfenv_t *xe, xdchange_t *xscr, xdemitcb_t *ecb,
>  		return xdl_emit_common(xe, xscr, ecb, xecfg);
>
>  	for (xch = xscr; xch; xch = xche->next) {
> -		xche = xdl_get_hunk(xch, xecfg);
> +		xche = xdl_get_hunk(&xch, xecfg);
> +		if (!xch)
> +			break;
>
>  		s1 = XDL_MAX(xch->i1 - xecfg->ctxlen, 0);
>  		s2 = XDL_MAX(xch->i2 - xecfg->ctxlen, 0);
> diff --git a/xdiff/xemit.h b/xdiff/xemit.h
> index c2e2e83..d297107 100644
> --- a/xdiff/xemit.h
> +++ b/xdiff/xemit.h
> @@ -27,7 +27,7 @@
>  typedef int (*emit_func_t)(xdfenv_t *xe, xdchange_t *xscr, xdemitcb_t *ecb,
>  			   xdemitconf_t const *xecfg);
>
> -xdchange_t *xdl_get_hunk(xdchange_t *xscr, xdemitconf_t const *xecfg);
> +xdchange_t *xdl_get_hunk(xdchange_t **xscr, xdemitconf_t const *xecfg);
>  int xdl_emit_diff(xdfenv_t *xe, xdchange_t *xscr, xdemitcb_t *ecb,
>  		  xdemitconf_t const *xecfg);
>
> diff --git a/xdiff/xutils.c b/xdiff/xutils.c
> index 9504eae..c047376 100644
> --- a/xdiff/xutils.c
> +++ b/xdiff/xutils.c
> @@ -143,6 +143,19 @@ long xdl_guess_lines(mmfile_t *mf, long sample) {
>  	return nl + 1;
>  }
>
> +int xdl_blankline(const char *line, long flags)
> +{
> +	long i;
> +
> +	if (!(flags & XDF_WHITESPACE_FLAGS))
> +		return (*line == '\n');
> +
> +	for (i = 0; line[i] != '\n' && XDL_ISSPACE(line[i]); i++)
> +		;
> +
> +	return (line[i] == '\n');
> +}
> +
>  int xdl_recmatch(const char *l1, long s1, const char *l2, long s2, long flags)
>  {
>  	int i1, i2;
> diff --git a/xdiff/xutils.h b/xdiff/xutils.h
> index ad1428e..b9cceff 100644
> --- a/xdiff/xutils.h
> +++ b/xdiff/xutils.h
> @@ -32,6 +32,7 @@ int xdl_cha_init(chastore_t *cha, long isize, long icount);
>  void xdl_cha_free(chastore_t *cha);
>  void *xdl_cha_alloc(chastore_t *cha);
>  long xdl_guess_lines(mmfile_t *mf, long sample);
> +int xdl_blankline(const char *line, long flags);
>  int xdl_recmatch(const char *l1, long s1, const char *l2, long s2, long flags);
>  unsigned long xdl_hash_record(char const **data, char const *top, long flags);
>  unsigned int xdl_hashbits(unsigned int size);
> --
> 1.7.9.5
--
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]