lkml.org 
[lkml]   [2019]   [Mar]   [6]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: [PATCH v2] scripts: checkpatch: Check multiple blank lines when deleting code
Date
On 3/5/19 6:17 PM, Joe Perches wrote:
> On Tue, 2019-03-05 at 02:12 -0500, Alexandre Ghiti wrote:
>> By matching only current line starting with '+', we miss the case
>> when deleting code makes consecutive blank lines appear: this patch
>> then makes it possible to detect this case by also matching current
>> line starting with ' ', which is an already existing blank line.
> []
>> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> []
>> @@ -2081,10 +2081,15 @@ sub fix_inserted_deleted_lines {
>> }
>>
>> while (defined($inserted) && ${$inserted}{'LINENR'} == $old_linenr) {
>> + my $len = 1;
>> push(@lines, ${$inserted}{'LINE'});
>> + # Do not increment the length when inserting a deletion line.
>> + if (${$inserted}{'LINE'} =~ /^\-/) {
>> + $len = 0;
>> + }
>> $inserted = @{$insertedRef}[$next_insert++];
>> $new_linenr++;
>> - fixup_current_range(\$lines[$range_last_linenr], $delta_offset++, 1);
>> + fixup_current_range(\$lines[$range_last_linenr], $delta_offset++, $len);
> I think this is confusing and unnecessary.
> Using --fix can not delete context lines from a patch.

Ok, I was expecting your comment on that, I agree with you.
So I'll get rid of the possibility to fix the deletion case, this will
only be a warning.

>> }
>>
>> if ($save_line) {
>> @@ -3298,12 +3303,18 @@ sub process {
>>
>> # check for multiple consecutive blank lines
>> if ($prevline =~ /^[\+ ]\s*$/ &&
>> - $line =~ /^\+\s*$/ &&
>> + $line =~ /^[\+ ]\s*$/ &&
>> $last_blank_line != ($linenr - 1)) {
>> if (CHK("LINE_SPACING",
>> "Please don't use multiple blank lines\n" . $hereprev) &&
>> $fix) {
> It's simpler to add a check that $line starts /^\+/ before $fix
>
> Maybe it'd be better to have a separate test for this to make
> the output message clearer too.
>
> Something like
> "Avoid deleting lines that create consecutive blank lines"

Ok,

Thanks for your comments,

>> fix_delete_line($fixlinenr, $rawline);
>> + if ($line =~ /^ \s*$/) {
>> + # If the line is not an inserted blank line, the multiple
>> + # consecutive blank lines are caused by deletion: fix this
>> + # by replacing the blank line with a deletion line.
>> + fix_insert_line($fixlinenr, "\-");
>> + }
>> }
>>
>> $last_blank_line = $linenr;

\
 
 \ /
  Last update: 2019-03-06 07:50    [W:0.171 / U:0.404 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site