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

[Jean Delvare]

Le vendredi 14 décembre 2012 à 15:36 +0100, Andreas Krebbel a écrit :
> 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: ".

Ah, good point. I have QUILT_NO_DIFF_INDEX=1 in my ~/.quiltrc so I
always forget about Index lines. This should make no difference in
practice though, the Index line will be ignored. Whether they are
ignored by the inner loop or the outer loop makes no difference. In fact
we could even ignore lines starting with "diff " (they are optional
anyway AFAIK, just as "Index:" lines), what really matters is to catch
the next line staring with "*** ".

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

-- 
Jean Delvare
Suse L3