diff mbox series

About Kconfig indentation fixes

Message ID 1236784830.606269.1697999064177@mail.yahoo.com (mailing list archive)
State New, archived
Headers show
Series About Kconfig indentation fixes | expand

Commit Message

Prasad Pandit Oct. 22, 2023, 6:24 p.m. UTC
Hello Masahiro,

Please see: -> https://paste.centos.org/view/06ed8bf0

===
+       depends on DMI
+       default y
        help
          Say Y here if you want to query SMBIOS/DMI system identification
          information from userspace through /sys/class/dmi/id/ or if you want 
===

* IIUC majority of Kconfig entries use leading tab(\t) character(s) for indentation. But there are multiple Kconfig entries like above wherein indentation is without the leading tab(\t) character. Ex. above entries use 4 spaces ("   ") for indentation.

* I wanted to check
  - is it okay to send fix patches for such inconsistencies? They may lead to errors.
  - if Kconfig file has no assigned maintainer(s), should the patch go to -kbuild@ list?

* The Kconfig language text below does not say anything about indentation or usage of leading tab(\t) characters. Is that intentional?

  https://www.kernel.org/doc/html/latest/kbuild/kconfig-language.html#menu-dependencies

* Is there some method/rule to the indentation in Kconfig files/entries?


Thank you.
---
  -Prasad

Comments

Randy Dunlap Oct. 23, 2023, 12:19 a.m. UTC | #1
On 10/22/23 11:24, P J P wrote:
> Hello Masahiro,
> 
> Please see: -> https://paste.centos.org/view/06ed8bf0
> 
> ===
> diff --git a/drivers/firmware/Kconfig b/drivers/firmware/Kconfig
> index b59e3041fd62..82ae68b13381 100644
> --- a/drivers/firmware/Kconfig
> +++ b/drivers/firmware/Kconfig
> @@ -68,14 +68,14 @@ config EDD_OFF
>           using the kernel parameter 'edd={on|skipmbr|off}'.
>  
>  config FIRMWARE_MEMMAP
> -    bool "Add firmware-provided memory map to sysfs" if EXPERT
> -    default X86
> -    help
> -      Add the firmware-provided (unmodified) memory map to /sys/firmware/memmap.
> -      That memory map is used for example by kexec to set up parameter area
> -      for the next kernel, but can also be used for debugging purposes.
> +       bool "Add firmware-provided memory map to sysfs" if EXPERT
> +       default X86
> +       help
> +         Add the firmware-provided (unmodified) memory map to /sys/firmware/memmap.
> +         That memory map is used for example by kexec to set up parameter area
> +         for the next kernel, but can also be used for debugging purposes.
>  
> -      See also Documentation/ABI/testing/sysfs-firmware-memmap.
> +         See also Documentation/ABI/testing/sysfs-firmware-memmap.
>   
>  config DMIID
> -    bool "Export DMI identification via sysfs to userspace"
> -    depends on DMI
> -    default y
> +       bool "Export DMI identification via sysfs to userspace"
> +       depends on DMI
> +       default y
>         help
>           Say Y here if you want to query SMBIOS/DMI system identification
>           information from userspace through /sys/class/dmi/id/ or if you want 
> ===
> 
> * IIUC majority of Kconfig entries use leading tab(\t) character(s) for indentation. But there are multiple Kconfig entries like above wherein indentation is without the leading tab(\t) character. Ex. above entries use 4 spaces ("   ") for indentation.
> 
> * I wanted to check
>   - is it okay to send fix patches for such inconsistencies? They may lead to errors.

IMO it just mucks up the git history for all of the affected lines.

>   - if Kconfig file has no assigned maintainer(s), should the patch go to -kbuild@ list?

Usually it is better to try to find out who has been merging the most recent patches
to the file (by using 'git log' to see the history).

> 
> * The Kconfig language text below does not say anything about indentation or usage of leading tab(\t) characters. Is that intentional?
> 

It's in coding-style.rst instead of kconfig-language.rst.

>   https://www.kernel.org/doc/html/latest/kbuild/kconfig-language.html#menu-dependencies
> 
> * Is there some method/rule to the indentation in Kconfig files/entries?

from Documentation/process/coding-style.rst:

10) Kconfig configuration files
-------------------------------

For all of the Kconfig* configuration files throughout the source tree,
the indentation is somewhat different.  Lines under a ``config`` definition
are indented with one tab, while help text is indented an additional two
spaces.  Example::
Prasad Pandit Oct. 23, 2023, 7:22 a.m. UTC | #2
Hi,

On Monday, 23 October, 2023 at 05:49:50 am IST, Randy Dunlap <rdunlap@infradead.org> wrote: 
>>On 10/22/23 11:24, P J P wrote:
>> Please see: -> https://paste.centos.org/view/06ed8bf0
>
>Usually it is better to try to find out who has been merging the most recent
>patches to the file (by using 'git log' to see the history).

* It's not always clear from history whom to contact. Some commits are quite old and original author's email has changed since then. Will check more...

>> * Is there some method/rule to the indentation in Kconfig files/entries?
>
>It's in coding-style.rst instead of kconfig-language.rst.
>
>from Documentation/process/coding-style.rst:
>
>10) Kconfig configuration files
>-------------------------------
>
>For all of the Kconfig* configuration files throughout the source tree,
>the indentation is somewhat different.  Lines under a ``config`` definition
>are indented with one tab, while help text is indented an additional two
>spaces.  Example::

* This is nice. It'll help to add this to the Kconfig language text as well. Also guidance if config entries within 'if' or 'choice' blocks should be indented with multiple tabs(\t) or if that's not recommended etc.


Thank you.
---
  -Prasad
diff mbox series

Patch

diff --git a/drivers/firmware/Kconfig b/drivers/firmware/Kconfig
index b59e3041fd62..82ae68b13381 100644
--- a/drivers/firmware/Kconfig
+++ b/drivers/firmware/Kconfig
@@ -68,14 +68,14 @@  config EDD_OFF
          using the kernel parameter 'edd={on|skipmbr|off}'.
 
 config FIRMWARE_MEMMAP
-    bool "Add firmware-provided memory map to sysfs" if EXPERT
-    default X86
-    help
-      Add the firmware-provided (unmodified) memory map to /sys/firmware/memmap.
-      That memory map is used for example by kexec to set up parameter area
-      for the next kernel, but can also be used for debugging purposes.
+       bool "Add firmware-provided memory map to sysfs" if EXPERT
+       default X86
+       help
+         Add the firmware-provided (unmodified) memory map to /sys/firmware/memmap.
+         That memory map is used for example by kexec to set up parameter area
+         for the next kernel, but can also be used for debugging purposes.
 
-      See also Documentation/ABI/testing/sysfs-firmware-memmap.
+         See also Documentation/ABI/testing/sysfs-firmware-memmap.
  
 config DMIID
-    bool "Export DMI identification via sysfs to userspace"
-    depends on DMI
-    default y
+       bool "Export DMI identification via sysfs to userspace"