Re: [PATCH v2] checkpatch: add double empty line check

On Tue, Nov 20, 2012 at 04:27:04PM +0200, Eilon Greenstein wrote:

On Tue, 2012-11-20 at 11:52 +0000, Andy Whitcroft wrote:

Andy, thanks for reviewing this patch.

quoted

On Sat, Nov 17, 2012 at 01:17:37PM +0200, Eilon Greenstein wrote:

quoted

Changes from previous attempt:
- Use CHK instead of WARN
- Issue only one warning per empty lines block

Signed-off-by: Eilon Greenstein <redacted>
---
 scripts/checkpatch.pl |    8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/scripts/checkpatch.pl b/scripts/linescheckpatch.pl
index 21a9f5d..13d264f 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl

@@ -3579,6 +3579,14 @@ sub process {
 			WARN("EXPORTED_WORLD_WRITABLE",
 			     "Exporting world writable files is usually an error. Consider more restrictive permissions.\n" . $herecurr);
 		}
+
+# check for double empty lines
+		if ($line =~ /^\+\s*$/ &&
+		    ($rawlines[$linenr] =~ /^\s*$/ ||
+		     $prevline =~ /^\+?\s*$/ && $rawlines[$linenr] !~ /^\+\s*$/)) {
+			CHK("DOUBLE_EMPTY_LINE",
+			    "One empty line should be sufficient. Consider removing this one.\n" . $herecurr);
+		}
 	}
 
 	# If we have no input at all, then there is nothing to report on

In your previous version you indicated you would be emiting one per group
of lines, I do not see how this does that.

This is what I'm testing:
Only if the current line is a new blank line and:
	if the next line is empty but not newly added (this is the part that
will make sure we get only one warning for a bunch of new lines - only
the last newly added line will hit this condition)
or
	if the previous line was empty (either new empty line or existing empty
line) and the next line is not a new empty line (so we will issue just
one warning).

I tested it on few examples, and did not see a problem. Can you share an
example where it issues more than a single warning for a newly
introduced consecutive new lines?

No indeed.  That was testing failure on my behalf.

quoted

Also this fails if the fragment
is at the top of the hunk emiting a perl warning.

I did not see this warning. Can you please share this example? I tried
adding a couple of empty lines at the beginning of a file and it seemed
to work just fine for me (using perl v5.14.2).lines

Ok, this is actually if it is at the bottom, not the top.  So if you
have a range of lines newly added to the bottom of the file.  Leading to
this warning:

Use of uninitialized value within @rawlines in pattern match (m//) at
../checkpatch/scripts/checkpatch.pl line 3586.

-apw