Messages in this thread | | | Subject | Re: [PATCH v2] scripts: checkpatch: Check multiple blank lines when deleting code | From | Joe Perches <> | Date | Tue, 05 Mar 2019 15:17:17 -0800 |
| |
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.
> } > > 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"
> 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;
| |