quilt-dev
[Top][All Lists]
Advanced

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

[Quilt-dev] diffstat patch


From: Jean Delvare
Subject: [Quilt-dev] diffstat patch
Date: Fri, 2 Sep 2005 19:17:19 +0200

Hi John,

I have merged part of your diffstat patch into quilt, and have comments
on the parts I did not merge.

> --- quilt.orig/quilt/refresh.in       2005-08-08 10:08:18.%N +1000
> +++ quilt/quilt/refresh.in    2005-08-17 02:07:21.%N +1000
> @@ -258,26 +258,27 @@
>  then
>       diffstat="$(@DIFFSTAT@ -p$opt_strip_level $tmp_patch)" || die 1
>       @AWK@ '
> -         function print_diffstat(arr, i) {
> +         function print_diffstat() {
>             split(diffstat, arr, "\n")
>             for (i=1; i in arr; i++)
>               print prefix arr[i]
>           }

Not merged, as it isn't correct. This change makes "arr" and "i" be
global variables rather than local variables, which doesn't make sense.
I agree that awk really has a strange syntax for declaring local
variable, but that's the way it is.

> -         BEGIN       { split(diffstat, ds_arr, /\n/) }

Merged, nice catch.

>                       { prefix=""
>                         if (index($0, "#") == 1)
>                           prefix="#"
>                       }
> -         /^#? .* \| /        { eat = eat $0 "\n"
> +         /^#? .* \|  *[1-9][0-9]* [+-][+-]*/ { eat = eat $0 "\n"
                          next }

I've made it more simple:

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

Yours wasn't correct (as explained in a previous post) and would fail to
catch alternative diffstat formats such as -f0. I don't think this will
be a problem, as a false positive is rather improbable. Having a "|"
followed by a number in a header line wouldn't be sufficient, this line
would also have to immediately precede a real diffstat section (without
even a blank line between them.) If such a case is ever reported, we'll
see, but until then this new regular expression looks like a good
tradeoff between the false positive risk and the flexibility towards
alternative diffstat formats.

>           /^#? .* files? changed(, .* insertions?\(\+\))?(, .* 
> deletions?\(-\))?/ \
>                       { print_diffstat()
>                         diffstat = "" ; eat = ""
>                         next }
>                       { print eat $0
> -                       eat = "" }
> +                       eat = ""
> +                       add_nl=(length($0) > 1) }
>           END         { printf "%s", eat
>                         if (diffstat != "") {
> +                         if (add_nl) print prefix
>                           print_diffstat()
>                           print prefix
>                         }

Not merged, as quilt isn't supposed to enforce any particular layout in
the header. The layout is up to the user, and tastes vary.

Thanks,
-- 
Jean Delvare




reply via email to

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