quilt-dev
[Top][All Lists]
Advanced

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

Re: [Quilt-dev] Re: config-cc patch


From: Jean Delvare
Subject: Re: [Quilt-dev] Re: config-cc patch
Date: Sun, 28 Aug 2005 22:54:59 +0200

Hi John,

[John Vandenberg]
> I think it would be useful to have a test to make sure the diffstat
> output appears where it should in the quilt patches, and that the awk
> script in refresh.in works as expected.  Can someone familiar with the
> code answer a few queries and take a look at the attached changes?
> 
> rev 1.45 allows diffstat output to begin with '#?' ; is this for
> compatibility with earlier versions?

I'd say it's simply for compatibility with diffstat's -c option:

  -c      prefix each line with comment (#)

> If I grok it correctly, the block "BEGIN { split(diffstat, ds_arr,
> /\n/) }" doesnt appear to be in use.

You seem to be right. Nice catch.

> The pattern /^#? .* \| / could accidentally match a pipe symbol used
> in the header; will the replacement work for the supported diffstat
> versions?

diffstat supports several output formats (see -f0, and -f4 in newest
versions), and I plan to make it possible to pick non-default ones in
quilt. Your change would prevent me from doing that, but I guess it can
be worked around by tweaking the regular expression again.

BTW, please note that your regular expression is not correct even for
the default format:

  /^#? .* \|  *[1-9][0-9]* [+-][+-]*/

Files can have empty bars if they have little changes and another file
in the same patch has a huge number of changes. Also, context diffs have
a third possible character, '!', for "modified lines". Last, you forgot
the end of line anchor. So the regular expression should read:

  /^#? .* \|  *[1-9][0-9]* [+-!]*$/

> Would anybody mind if quilt added a blank line between the header and
> the output of diffstat if the last line of the header contained text?

What would be the rationale for doing so? Users should be free to layout
their headers exactly as they see fit.

> Finally, does anyone object to a new simple test diffstat.test being
> added as an optional test?  I have taken care to avoid including
> complex histograms in the test case to avoid the test case breaking
> when the diffstat algorithms are updated.

Fine with me, but how will you make this test case optional?

Thanks,
-- 
Jean Delvare




reply via email to

[Prev in Thread] Current Thread [Next in Thread]