* [PATCH] checkpatch: Look for Kconfig indentation errors @ 2019-11-28 2:06 Krzysztof Kozlowski 2019-11-28 9:29 ` Jani Nikula 0 siblings, 1 reply; 8+ messages in thread From: Krzysztof Kozlowski @ 2019-11-28 2:06 UTC (permalink / raw) To: Andy Whitcroft, Joe Perches, linux-kernel Cc: Jani Nikula, Pierre-Louis Bossart, Krzysztof Kozlowski Kconfig should be indented with one tab for first level and tab+2 spaces for second level. There are many mixups of this so add a checkpatch rule. Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org> --- scripts/checkpatch.pl | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index e41f4adcc1be..875e862cf076 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -3046,6 +3046,13 @@ sub process { "Use of boolean is deprecated, please use bool instead.\n" . $herecurr); } +# Kconfig has special indentation + if ($realfile =~ /Kconfig/ && + ($rawline =~ /^\+ +\t* *[a-zA-Z-]/) || ($rawline =~ /^\+\t( | )[a-zA-Z-]/)) { + WARN("CONFIG_INDENTATION", + "Kconfig uses one tab indentation, optionally followed by two spaces.\n" . $herecurr); + } + if (($realfile =~ /Makefile.*/ || $realfile =~ /Kbuild.*/) && ($line =~ /\+(EXTRA_[A-Z]+FLAGS).*/)) { my $flag = $1; -- 2.7.4 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] checkpatch: Look for Kconfig indentation errors 2019-11-28 2:06 [PATCH] checkpatch: Look for Kconfig indentation errors Krzysztof Kozlowski @ 2019-11-28 9:29 ` Jani Nikula 2019-11-28 9:34 ` Joe Perches 0 siblings, 1 reply; 8+ messages in thread From: Jani Nikula @ 2019-11-28 9:29 UTC (permalink / raw) To: Krzysztof Kozlowski, Andy Whitcroft, Joe Perches, linux-kernel Cc: Pierre-Louis Bossart, Krzysztof Kozlowski On Thu, 28 Nov 2019, Krzysztof Kozlowski <krzk@kernel.org> wrote: > Kconfig should be indented with one tab for first level and tab+2 spaces > for second level. There are many mixups of this so add a checkpatch > rule. > > Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org> I agree unifying the indentation is nice, and without something like this it'll start bitrotting before Krzysztof's done fixing them all... I think there's been quite a few fixes merged lately. I approve of the idea, but I'm clueless about the implementation. BR, Jani. > --- > scripts/checkpatch.pl | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl > index e41f4adcc1be..875e862cf076 100755 > --- a/scripts/checkpatch.pl > +++ b/scripts/checkpatch.pl > @@ -3046,6 +3046,13 @@ sub process { > "Use of boolean is deprecated, please use bool instead.\n" . $herecurr); > } > > +# Kconfig has special indentation > + if ($realfile =~ /Kconfig/ && > + ($rawline =~ /^\+ +\t* *[a-zA-Z-]/) || ($rawline =~ /^\+\t( | )[a-zA-Z-]/)) { > + WARN("CONFIG_INDENTATION", > + "Kconfig uses one tab indentation, optionally followed by two spaces.\n" . $herecurr); > + } > + > if (($realfile =~ /Makefile.*/ || $realfile =~ /Kbuild.*/) && > ($line =~ /\+(EXTRA_[A-Z]+FLAGS).*/)) { > my $flag = $1; -- Jani Nikula, Intel Open Source Graphics Center ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] checkpatch: Look for Kconfig indentation errors 2019-11-28 9:29 ` Jani Nikula @ 2019-11-28 9:34 ` Joe Perches 2019-12-03 8:40 ` Krzysztof Kozlowski 0 siblings, 1 reply; 8+ messages in thread From: Joe Perches @ 2019-11-28 9:34 UTC (permalink / raw) To: Jani Nikula, Krzysztof Kozlowski, Andy Whitcroft, linux-kernel Cc: Pierre-Louis Bossart On Thu, 2019-11-28 at 11:29 +0200, Jani Nikula wrote: > On Thu, 28 Nov 2019, Krzysztof Kozlowski <krzk@kernel.org> wrote: > > Kconfig should be indented with one tab for first level and tab+2 spaces > > for second level. There are many mixups of this so add a checkpatch > > rule. > > > > Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org> > > I agree unifying the indentation is nice, and without something like > this it'll start bitrotting before Krzysztof's done fixing them all... I > think there's been quite a few fixes merged lately. > > I approve of the idea, but I'm clueless about the implementation. I think that a grammar, or a least an array of words that are supposed to start on a tab should be used here. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] checkpatch: Look for Kconfig indentation errors 2019-11-28 9:34 ` Joe Perches @ 2019-12-03 8:40 ` Krzysztof Kozlowski 2019-12-03 8:55 ` Krzysztof Kozlowski 2019-12-03 8:56 ` Joe Perches 0 siblings, 2 replies; 8+ messages in thread From: Krzysztof Kozlowski @ 2019-12-03 8:40 UTC (permalink / raw) To: Joe Perches Cc: Jani Nikula, Andy Whitcroft, linux-kernel, Pierre-Louis Bossart On Thu, 28 Nov 2019 at 17:35, Joe Perches <joe@perches.com> wrote: > > On Thu, 2019-11-28 at 11:29 +0200, Jani Nikula wrote: > > On Thu, 28 Nov 2019, Krzysztof Kozlowski <krzk@kernel.org> wrote: > > > Kconfig should be indented with one tab for first level and tab+2 spaces > > > for second level. There are many mixups of this so add a checkpatch > > > rule. > > > > > > Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org> > > > > I agree unifying the indentation is nice, and without something like > > this it'll start bitrotting before Krzysztof's done fixing them all... I > > think there's been quite a few fixes merged lately. > > > > I approve of the idea, but I'm clueless about the implementation. > > I think that a grammar, or a least an array of words > that are supposed to start on a tab should be used here. This won't work for wrong indentation of help text. This is quite popular Kconfig indentation violation so worth checking. I can then check for: 1. any white-space violations before array of Kconfig words - that 2. spaces mixed with tab before any text, 3. just spaces before any text, 4. tab + wrong number of spaces before any text. It would look like: + if ($realfile =~ /Kconfig/ && + (($rawline =~ /^\+\s+(?:config|menuconfig|choice|endchoice|if|endif|menu|endmenu|source|bool|tristate|prompt|help|---help---|depends|select)\b/ && + $rawline !~ /^\+\t[a-z-]/) || + $rawline =~ /^\+\t* +\t+ *[a-zA-Z0-9-]/ || + $rawline =~ /^\+\t( | )[a-zA-Z0-9-]/)) { Best regards, Krzysztof ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] checkpatch: Look for Kconfig indentation errors 2019-12-03 8:40 ` Krzysztof Kozlowski @ 2019-12-03 8:55 ` Krzysztof Kozlowski 2019-12-03 8:56 ` Joe Perches 1 sibling, 0 replies; 8+ messages in thread From: Krzysztof Kozlowski @ 2019-12-03 8:55 UTC (permalink / raw) To: Joe Perches Cc: Jani Nikula, Andy Whitcroft, linux-kernel, Pierre-Louis Bossart On Tue, 3 Dec 2019 at 16:40, Krzysztof Kozlowski <krzk@kernel.org> wrote: > > On Thu, 28 Nov 2019 at 17:35, Joe Perches <joe@perches.com> wrote: > > > > On Thu, 2019-11-28 at 11:29 +0200, Jani Nikula wrote: > > > On Thu, 28 Nov 2019, Krzysztof Kozlowski <krzk@kernel.org> wrote: > > > > Kconfig should be indented with one tab for first level and tab+2 spaces > > > > for second level. There are many mixups of this so add a checkpatch > > > > rule. > > > > > > > > Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org> > > > > > > I agree unifying the indentation is nice, and without something like > > > this it'll start bitrotting before Krzysztof's done fixing them all... I > > > think there's been quite a few fixes merged lately. > > > > > > I approve of the idea, but I'm clueless about the implementation. > > > > I think that a grammar, or a least an array of words > > that are supposed to start on a tab should be used here. > > This won't work for wrong indentation of help text. This is quite > popular Kconfig indentation violation so worth checking. I can then > check for: > 1. any white-space violations before array of Kconfig words - that > 2. spaces mixed with tab before any text, > 3. just spaces before any text, > 4. tab + wrong number of spaces before any text. > > It would look like: > + if ($realfile =~ /Kconfig/ && > + (($rawline =~ > /^\+\s+(?:config|menuconfig|choice|endchoice|if|endif|menu|endmenu|source|bool|tristate|prompt|help|---help---|depends|select)\b/ > && > + $rawline !~ /^\+\t[a-z-]/) || > + $rawline =~ /^\+\t* +\t+ *[a-zA-Z0-9-]/ || > + $rawline =~ /^\+\t( | )[a-zA-Z0-9-]/)) { This unfortunately fails if help text starts with one of syntax keywords (e.g. "if"). Isn't this getting over-complicated? The Kconfig is rather simple: 1. no indentation, 2. one tab, 3. one tab + 2 spaces 4. one tab + 2 spaces + some more spaces (e.g. help text) Best regards, Krzysztof ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] checkpatch: Look for Kconfig indentation errors 2019-12-03 8:40 ` Krzysztof Kozlowski 2019-12-03 8:55 ` Krzysztof Kozlowski @ 2019-12-03 8:56 ` Joe Perches 2019-12-03 9:23 ` Jani Nikula 1 sibling, 1 reply; 8+ messages in thread From: Joe Perches @ 2019-12-03 8:56 UTC (permalink / raw) To: Krzysztof Kozlowski Cc: Jani Nikula, Andy Whitcroft, linux-kernel, Pierre-Louis Bossart On Tue, 2019-12-03 at 16:40 +0800, Krzysztof Kozlowski wrote: > On Thu, 28 Nov 2019 at 17:35, Joe Perches <joe@perches.com> wrote: > > On Thu, 2019-11-28 at 11:29 +0200, Jani Nikula wrote: > > > On Thu, 28 Nov 2019, Krzysztof Kozlowski <krzk@kernel.org> wrote: > > > > Kconfig should be indented with one tab for first level and tab+2 spaces > > > > for second level. There are many mixups of this so add a checkpatch > > > > rule. > > > > > > > > Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org> > > > > > > I agree unifying the indentation is nice, and without something like > > > this it'll start bitrotting before Krzysztof's done fixing them all... I > > > think there's been quite a few fixes merged lately. > > > > > > I approve of the idea, but I'm clueless about the implementation. > > > > I think that a grammar, or a least an array of words > > that are supposed to start on a tab should be used here. > > This won't work for wrong indentation of help text. This is quite > popular Kconfig indentation violation so worth checking. I can then > check for: > 1. any white-space violations before array of Kconfig words - that > 2. spaces mixed with tab before any text, > 3. just spaces before any text, > 4. tab + wrong number of spaces before any text. > > It would look like: > + if ($realfile =~ /Kconfig/ && > + (($rawline =~ > /^\+\s+(?:config|menuconfig|choice|endchoice|if|endif|menu|endmenu|source|bool|tristate|prompt|help|---help---|depends|select)\b/ Many of these are not correct. config, menuconfig, choice, endchoice, source are primarily used at the beginning of a line. if is odd as it's a logical block or test It really needs a lex grammar to work properly. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] checkpatch: Look for Kconfig indentation errors 2019-12-03 8:56 ` Joe Perches @ 2019-12-03 9:23 ` Jani Nikula 2019-12-03 9:38 ` Joe Perches 0 siblings, 1 reply; 8+ messages in thread From: Jani Nikula @ 2019-12-03 9:23 UTC (permalink / raw) To: Joe Perches, Krzysztof Kozlowski Cc: Andy Whitcroft, linux-kernel, Pierre-Louis Bossart On Tue, 03 Dec 2019, Joe Perches <joe@perches.com> wrote: > On Tue, 2019-12-03 at 16:40 +0800, Krzysztof Kozlowski wrote: >> On Thu, 28 Nov 2019 at 17:35, Joe Perches <joe@perches.com> wrote: >> > On Thu, 2019-11-28 at 11:29 +0200, Jani Nikula wrote: >> > > On Thu, 28 Nov 2019, Krzysztof Kozlowski <krzk@kernel.org> wrote: >> > > > Kconfig should be indented with one tab for first level and tab+2 spaces >> > > > for second level. There are many mixups of this so add a checkpatch >> > > > rule. >> > > > >> > > > Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org> >> > > >> > > I agree unifying the indentation is nice, and without something like >> > > this it'll start bitrotting before Krzysztof's done fixing them all... I >> > > think there's been quite a few fixes merged lately. >> > > >> > > I approve of the idea, but I'm clueless about the implementation. >> > >> > I think that a grammar, or a least an array of words >> > that are supposed to start on a tab should be used here. >> >> This won't work for wrong indentation of help text. This is quite >> popular Kconfig indentation violation so worth checking. I can then >> check for: >> 1. any white-space violations before array of Kconfig words - that >> 2. spaces mixed with tab before any text, >> 3. just spaces before any text, >> 4. tab + wrong number of spaces before any text. >> >> It would look like: >> + if ($realfile =~ /Kconfig/ && >> + (($rawline =~ >> /^\+\s+(?:config|menuconfig|choice|endchoice|if|endif|menu|endmenu|source|bool|tristate|prompt|help|---help---|depends|select)\b/ > > Many of these are not correct. > > config, menuconfig, choice, endchoice, source > are primarily used at the beginning of a line. > > if is odd as it's a logical block or test > > It really needs a lex grammar to work properly. Alternatively, perhaps you could complain about indentation that is not one of 1) empty string, 2) exactly one tab, or 3) exactly one tab followed by exactly two spaces? BR, Jani. -- Jani Nikula, Intel Open Source Graphics Center ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] checkpatch: Look for Kconfig indentation errors 2019-12-03 9:23 ` Jani Nikula @ 2019-12-03 9:38 ` Joe Perches 0 siblings, 0 replies; 8+ messages in thread From: Joe Perches @ 2019-12-03 9:38 UTC (permalink / raw) To: Jani Nikula, Krzysztof Kozlowski Cc: Andy Whitcroft, linux-kernel, Pierre-Louis Bossart On Tue, 2019-12-03 at 11:23 +0200, Jani Nikula wrote: > Alternatively, perhaps you could complain about indentation that is not > one of 1) empty string, 2) exactly one tab, or 3) exactly one tab > followed by exactly two spaces? Way too many false positives. Try something like: $ git grep -P -oh "^\s*\w+\b" -- '*/Kconfig*' | \ perl -p -e 'my $tabs=0;my $spaces=0;while ($_ =~ /^\s/) { if (substr($_,0,1) eq " ") { $spaces++; } else { $tabs++; } $_ = substr($_, 1); } print "tabs: $tabs spaces: $spaces: word: $_";' ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2019-12-03 9:39 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-11-28 2:06 [PATCH] checkpatch: Look for Kconfig indentation errors Krzysztof Kozlowski 2019-11-28 9:29 ` Jani Nikula 2019-11-28 9:34 ` Joe Perches 2019-12-03 8:40 ` Krzysztof Kozlowski 2019-12-03 8:55 ` Krzysztof Kozlowski 2019-12-03 8:56 ` Joe Perches 2019-12-03 9:23 ` Jani Nikula 2019-12-03 9:38 ` Joe Perches
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).