linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jani Nikula <jani.nikula@linux.intel.com>
To: Joe Perches <joe@perches.com>, Krzysztof Kozlowski <krzk@kernel.org>
Cc: Andy Whitcroft <apw@canonical.com>,
	"linux-kernel\@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Subject: Re: [PATCH] checkpatch: Look for Kconfig indentation errors
Date: Tue, 03 Dec 2019 11:23:33 +0200	[thread overview]
Message-ID: <874kyhkbje.fsf@intel.com> (raw)
In-Reply-To: <ea57f41e30f962227855d4f60a93c89a6bf0b2f0.camel@perches.com>

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

  reply	other threads:[~2019-12-03  9:23 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 [this message]
2019-12-03  9:38           ` Joe Perches

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=874kyhkbje.fsf@intel.com \
    --to=jani.nikula@linux.intel.com \
    --cc=apw@canonical.com \
    --cc=joe@perches.com \
    --cc=krzk@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=pierre-louis.bossart@linux.intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).