diff mbox

[RFC] checkpatch: check for 2 or more spaces around assignment of a declaration

Message ID 1432170126.20840.22.camel@perches.com
State Not Applicable
Headers show

Commit Message

Joe Perches May 21, 2015, 1:02 a.m. UTC 
On Wed, 2015-05-20 at 19:59 +0200, Jean Delvare wrote:
> On Wed, 20 May 2015 22:44:52 +0530, Sudip Mukherjee wrote:
> > On Wed, May 20, 2015 at 05:49:07PM +0200, Wolfram Sang wrote:
> > > On Wed, May 20, 2015 at 08:57:00PM +0530, Sudip Mukherjee wrote:
> > > >  static struct parport_driver i2c_parport_driver = {
> > > > -	.name	= "i2c-parport",
> > > > -	.attach	= i2c_parport_attach,
> > > > -	.detach	= i2c_parport_detach,
> > > > +	.name		= "i2c-parport",
> > > > +	.match_port	= i2c_parport_attach,
> > > > +	.detach		= i2c_parport_detach,
> > > > +	.devmodel	= true,
> > > 
> > > Minor nit: I prefer to not use tabs but a single space after the struct
> > > member names. Less hazzle in the future and still readable IMO.
> >
> > It was having space originally. I changed that into tab as it was
> > looking good with them as aligned.
> 
> As the driver maintainer, I am fine with both unaligned or tab-aligned.
> Space-aligned as I did originally was not a good idea, I admit.

Perhaps space aligned declarations should have some
checkpatch --strict warning for 2 or more spaces
around any assignment of the declaration.

	int a   = 1;		// 2+ spaces before =
	int b	=  2;		// 2+ spaces after =
	int c	=	3;	// uses tabs around =, no warning
	int d		= 4;	// uses 2 tabs before =, no warning
	
Something like:
---
 scripts/checkpatch.pl | 7 +++++++
 1 file changed, 7 insertions(+)




--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Willy Tarreau May 21, 2015, 5:48 a.m. UTC | #1 
On Wed, May 20, 2015 at 06:02:06PM -0700, Joe Perches wrote:
> Perhaps space aligned declarations should have some
> checkpatch --strict warning for 2 or more spaces
> around any assignment of the declaration.
> 
> 	int a   = 1;		// 2+ spaces before =
> 	int b	=  2;		// 2+ spaces after =
> 	int c	=	3;	// uses tabs around =, no warning
> 	int d		= 4;	// uses 2 tabs before =, no warning

FWIW, in my own code I've stopped using tabs for this and replaced
them with spaces. Tabs are fine *before* code but they completely
mangle the display for people using different tab sizes, or even
when reading diffs, because they heavily depend on the number of
characters *before* them while what you want is to ensure that
what is *after* is on a fixed position.

I would even go further and report warnings when tabs are used after
text.

Tabs after text were very useful in ASM editors in the past because
it was possible to define 3 fixed positions (mnemonic, operands,
comments) and that was the primary purpose of tabs. But in todays
editors, tabs do not mean "jump to next position" but "add between
1 and 8 to the current position" which doesn't make sense at all
if what preceeds can be larger than 8 (which is most often the case
in C code).

Willy

--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 89b1df4..8f9d26f 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -3902,6 +3902,13 @@  sub process {
 			    "multiple assignments should be avoided\n" . $herecurr);
 		}
 
+# check for space aligned declarations
+		if ($line =~ /^.\s*(?:$Declare|$DeclareMisordered)\s*$Ident {2,}=\s*(?:$Lval|$Constant|$String)/ ||
+		    $line =~ /^.\s*(?:$Declare|$DeclareMisordered)\s*$Ident\s*= {2,}(?:$Lval|$Constant|$String)/) {
+			CHK("SPACING",
+			    "Avoid multiple space assigments - prefer a single space or tabs\n" . $herecurr);
+		}
+
 ## # check for multiple declarations, allowing for a function declaration
 ## # continuation.
 ## 		if ($line =~ /^.\s*$Type\s+$Ident(?:\s*=[^,{]*)?\s*,\s*$Ident.*/ &&