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

[Jean Delvare]

Hi Andreas,

[Jean Delvare]
> When using the --strip-trailing-whitespace option of quilt refresh,
> the patch may not match the current state. I think this caused some
> trouble to me already (I had to force pop commands). I stopped using
> this option because of that. I think this is a problem, but have no
> idea how it can be solved. Anyone wants to comment on this?

[Andreas Gruenbacher]
> Well, one fix would be to strip the whitespace from the files in the
> working  tree instead of from the generated patch. It's a bit nasty:
> we don't want to  strip whitespace from all lines or else we would end
> up with noise form  whitespace-unclean files. Attached is a first
> attempt to do that.

Nice script. Here are a few comments:

> # Remove trailing whitespace from modified lines in a workign file

Typo on "working".

> my $dfh = new FileHandle("< $diff")
>     or die "$ARGV[1]: $!\n";

I'd use $diff rather than $ARGV[1] for clarity.

>     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? 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...

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?

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)?

Thanks,
-- 
Jean Delvare