Re: [Quilt-dev] [PATCH] remove-trailing-ws

[Andreas Krebbel]

On 14/12/12 15:13, Jean Delvare wrote:
> Hi again Andreas,
> 
> Le jeudi 13 décembre 2012 à 18:27 +0100, Jean Delvare a écrit :
>> Le vendredi 30 novembre 2012 à 10:18 +0100, Andreas Krebbel a écrit :
>>> the script for trailing whitespace removal fails for me if a context diff 
>>> has an "delete-only" chunk
>>> at the end. The following patch fixes this for me.
>>
>> This is because apparently chunk halves which contain no changes can be
>> omitted from context diffs.
>>
>> Didn't know context diffs were still in use somewhere...
>>
>>>  quilt/scripts/remove-trailing-ws.in |    3 +--
>>>  1 file changed, 1 insertion(+), 2 deletions(-)
>>>
>>> Index: quilt-0.48/quilt/scripts/remove-trailing-ws.in
>>> ===================================================================
>>> --- quilt-0.48.orig/quilt/scripts/remove-trailing-ws.in
>>> +++ quilt-0.48/quilt/scripts/remove-trailing-ws.in
>>> @@ -88,8 +88,7 @@ while (<>) {
>>>             my $last_line = defined $2 ? $2 : $1;
>>>             while ($line_number <= $last_line) {
>>>                 $_ = <>;
>>> -               defined $_
>>> -                   or die sprintf(_("%s: I'm confused.\n"), $0);
>>> +               defined $_ or last;
>>>                 if (s/(^[+!] .*?)[ \t]+$/$1/) {
>>>                     push @{$files{$file}}, $line_number;
>>>                 }
>>>
>>
>> Looks reasonable. Applied, thanks.
> 
> Actually I found that your fix is needed but not sufficient. It works
> fine if the hunk which is only removing lines is at the end of the file.
> However, if it is not and the hunk immediately after is adding lines
> with trailing whitespace to another file, the script will get the file
> name wrong. It was already the case before, and doesn't fail with "I'm
> confused", just the output is wrong. So I have changed your fix to:
> 
> --- a/quilt/scripts/remove-trailing-ws.in
> +++ b/quilt/scripts/remove-trailing-ws.in
> @@ -88,8 +88,8 @@ while (<>) {
>               my $last_line = defined $2 ? $2 : $1;
>               while ($line_number <= $last_line) {
>                   $_ = <>;
> -                 defined $_
> -                     or die sprintf(_("%s: I'm confused.\n"), $0);
> +                 defined $_ or last;
> +                 last if /^diff / || /^\*\*\* /;
>                   if (s/(^[+!] .*?)[ \t]+$/$1/) {
>                       push @{$files{$file}}, $line_number;
>                   }
> 
> Is it OK with you?

Right that's missing. But why do you check for ^diff ? For a context diff I 
would expect the line
after the empty chunk to start with "Index: ".

Feel free to adjust the patch as you want. Thanks!

Bye,

-Andreas-