Re: [Quilt-dev] merging with upstream and adding new files

[Andreas Gruenbacher]

On Sunday 12 June 2005 21:40, Jean Delvare wrote:
> >     if (/^@@ -(\d+)(?:,(\d+)) \+(\d+)(?:,(\d+)) @@/) {
> >     my ($ln, $n, $lr) = ($2 || 1, $3, $4 || 1);
>
> I'm a bit confused by this regular expression. Why are you using the
> "?:" construct?

The 2nd and 4th parentheses are not counted with (?:...); they are used for 
grouping only.

> Also, how could $2 or $4 be undefined? To me, the following is equivalent 
and more efficient:
>
>     if (/^@@ -\d+,(\d+) \+(\d+),(\d+) @@/) {
>       my ($ln, $n, $lr) = ($1, $2, $3);
>
> But I am certainly missing something...

The line ranges can be of the form \d+ or \d+,\d+. Example:

        diff -U0 <(echo 'a') <(echo 'b'; echo 'c')
        [...]
        @@ -1 +1,2 @@
        -a
        +b
        +c

My first version got the 0 case wrong though.

> Also, I think that the name $n is somewhat error prone, as it works with
> $lr, not $ln (which themselves aren't exactly explicit variable names).

;)

> >     print "Removing trailing whitespace from line(s) ",
> >       join(', ', (keys %lines)), " of $file\n";
>
> Sorting the keys would be more user friendly. Also, I am not sure that
> using a hash for this is good. Since you're adding the line numbers in
> order, and need them in the same order, a simple list and push+shift
> should work, and should be more efficient. Unless unified diffs are
> allowed not have their changes in random order?

Lists are somewhat simpler here, yes.

> I am also wondering if a forced pop followed by a push when a refresh
> strips whitespace wouldn't be just as efficient, and cheaper (as far as
> code goes)?

I'd rather not. The script has a fair chance of doing the right thing when 
refreshing a patch that is not on top. With pop+push this would get nasty.

-- Andreas.