linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3] Add .editorconfig file for basic formatting
@ 2023-04-14 10:10 Íñigo Huguet
  2023-04-14 13:36 ` Miguel Ojeda
  2023-05-16 13:04 ` Vincent Mailhol
  0 siblings, 2 replies; 10+ messages in thread
From: Íñigo Huguet @ 2023-04-14 10:10 UTC (permalink / raw)
  To: ojeda, masahiroy, jgg, mic, danny
  Cc: linux-kernel, corbet, joe, linux, willy, Íñigo Huguet

EditorConfig is a specification to define the most basic code formatting
stuff, and it's supported by many editors and IDEs, either directly or
via plugins, including VSCode/VSCodium, Vim, emacs and more.

It allows to define formatting style related to indentation, charset,
end of lines and trailing whitespaces. It also allows to apply different
formats for different files based on wildcards, so for example it is
possible to apply different configs to *.{c,h}, *.py and *.rs.

In linux project, defining a .editorconfig might help to those people
that work on different projects with different indentation styles, so
they cannot define a global style. Now they will directly see the
correct indentation on every fresh clone of the project.

See https://editorconfig.org

Link: https://lore.kernel.org/lkml/20200703073143.423557-1-danny@kdrag0n.dev/
Link: https://lore.kernel.org/lkml/20230404075540.14422-1-ihuguet@redhat.com/
Co-developed-by: Danny Lin <danny@kdrag0n.dev>
Signed-off-by: Danny Lin <danny@kdrag0n.dev>
Signed-off-by: Íñigo Huguet <ihuguet@redhat.com>
---
v2:
 - added special rule for patch files so it doesn't remove
   trailing whitespaces, making them unusable.
v3:
 - moved all rules from [*] section to all the individual
   sections so they doesn't affect to unexpected files.
 - added some extensions and files from a patch from Danny
   Lin that didn't get to be merged:
   https://lore.kernel.org/lkml/20200703073143.423557-1-danny@kdrag0n.dev/
   However, the following file types hasn't been added
   because they don't have a clear common style:
   rst,pl,cocci,tc,bconf,svg,xsl,manual pages
---
 .editorconfig                          | 30 ++++++++++++++++++++++++++
 .gitignore                             |  1 +
 Documentation/process/4.Coding.rst     |  4 ++++
 Documentation/process/coding-style.rst |  4 ++++
 4 files changed, 39 insertions(+)
 create mode 100644 .editorconfig

diff --git a/.editorconfig b/.editorconfig
new file mode 100644
index 000000000000..dce20d45c246
--- /dev/null
+++ b/.editorconfig
@@ -0,0 +1,30 @@
+# SPDX-License-Identifier: GPL-2.0-only
+
+root = true
+
+# 8 width tabs
+[{*.{c,h},Kconfig,Makefile,Makefile.*,*.mk}]
+charset = utf-8
+end_of_line = lf
+trim_trailing_whitespace = true
+insert_final_newline = true
+indent_style = tab
+indent_size = 8
+
+# 4 spaces
+[{*.{json,pm,py,rs},tools/perf/scripts/*/bin/*}]
+charset = utf-8
+end_of_line = lf
+trim_trailing_whitespace = true
+insert_final_newline = true
+indent_style = space
+indent_size = 4
+
+# 2 spaces
+[{*.{rb,yaml},.clang-format}]
+charset = utf-8
+end_of_line = lf
+trim_trailing_whitespace = true
+insert_final_newline = true
+indent_style = space
+indent_size = 2
diff --git a/.gitignore b/.gitignore
index 70ec6037fa7a..e4b3fe1d029b 100644
--- a/.gitignore
+++ b/.gitignore
@@ -100,6 +100,7 @@ modules.order
 #
 !.clang-format
 !.cocciconfig
+!.editorconfig
 !.get_maintainer.ignore
 !.gitattributes
 !.gitignore
diff --git a/Documentation/process/4.Coding.rst b/Documentation/process/4.Coding.rst
index 1f0d81f44e14..c2046dec0c2f 100644
--- a/Documentation/process/4.Coding.rst
+++ b/Documentation/process/4.Coding.rst
@@ -66,6 +66,10 @@ for aligning variables/macros, for reflowing text and other similar tasks.
 See the file :ref:`Documentation/process/clang-format.rst <clangformat>`
 for more details.
 
+Some basic editor settings, such as indentation and line endings, will be
+set automatically if you are using an editor that is compatible with
+EditorConfig. See the official EditorConfig website for more information:
+https://editorconfig.org/
 
 Abstraction layers
 ******************
diff --git a/Documentation/process/coding-style.rst b/Documentation/process/coding-style.rst
index 007e49ef6cec..ec96462fa8be 100644
--- a/Documentation/process/coding-style.rst
+++ b/Documentation/process/coding-style.rst
@@ -735,6 +735,10 @@ for aligning variables/macros, for reflowing text and other similar tasks.
 See the file :ref:`Documentation/process/clang-format.rst <clangformat>`
 for more details.
 
+Some basic editor settings, such as indentation and line endings, will be
+set automatically if you are using an editor that is compatible with
+EditorConfig. See the official EditorConfig website for more information:
+https://editorconfig.org/
 
 10) Kconfig configuration files
 -------------------------------
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH v3] Add .editorconfig file for basic formatting
  2023-04-14 10:10 [PATCH v3] Add .editorconfig file for basic formatting Íñigo Huguet
@ 2023-04-14 13:36 ` Miguel Ojeda
  2023-05-08  8:58   ` Íñigo Huguet
  2023-05-16 13:04 ` Vincent Mailhol
  1 sibling, 1 reply; 10+ messages in thread
From: Miguel Ojeda @ 2023-04-14 13:36 UTC (permalink / raw)
  To: Íñigo Huguet
  Cc: ojeda, masahiroy, jgg, mic, danny, linux-kernel, corbet, joe,
	linux, willy, Andrew Morton

On Fri, Apr 14, 2023 at 12:11 PM Íñigo Huguet <ihuguet@redhat.com> wrote:
>
> EditorConfig is a specification to define the most basic code formatting
> stuff, and it's supported by many editors and IDEs, either directly or
> via plugins, including VSCode/VSCodium, Vim, emacs and more.

Thanks -- v3 looks much safer!

To clarify the risks (it would be nice to detail these in the commit message):

  - Did you sample files manually or did you automate the search (e.g.
grepping for spaces/tabs, for LF, etc.) to verify the current rules
match the files in the kernel tree?

  - Would it be possible to go further than grepping and apply the
rules (e.g. trigger a "save") through the entire tree to see whether
there would be spurious changes?

    If that comes out clean (or mostly clean), then we would be fairly
confident this will not surprise developers (and it would be nice to
have the script around for future updates of the `.editorconfig`).

    Perhaps EditorConfig provides a script to check this already?
Otherwise perhaps it can be done with editorconfig-core-c or
editorconfig-vim or directly scripting on a couple editors?

  - Are we sure the rules match the output of automated formatters we
are using? (e.g. for Rust we enforce `rustfmt`, and thus we need to
ensure the editor does not "fight" the formatter; otherwise developers
may need to run the formatter more).

Cc'ing Andrew since he applied originally the `.clang-format`.

Cheers,
Miguel

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v3] Add .editorconfig file for basic formatting
  2023-04-14 13:36 ` Miguel Ojeda
@ 2023-05-08  8:58   ` Íñigo Huguet
  2023-05-26 21:16     ` Miguel Ojeda
  0 siblings, 1 reply; 10+ messages in thread
From: Íñigo Huguet @ 2023-05-08  8:58 UTC (permalink / raw)
  To: Miguel Ojeda
  Cc: ojeda, masahiroy, jgg, mic, danny, linux-kernel, corbet, joe,
	linux, willy, Andrew Morton

Hi, I finally had a chance to look at this.

On Fri, Apr 14, 2023 at 3:36 PM Miguel Ojeda
<miguel.ojeda.sandonis@gmail.com> wrote:
>
> On Fri, Apr 14, 2023 at 12:11 PM Íñigo Huguet <ihuguet@redhat.com> wrote:
> >
> > EditorConfig is a specification to define the most basic code formatting
> > stuff, and it's supported by many editors and IDEs, either directly or
> > via plugins, including VSCode/VSCodium, Vim, emacs and more.
>
> Thanks -- v3 looks much safer!
>
> To clarify the risks (it would be nice to detail these in the commit message):
>
>   - Did you sample files manually or did you automate the search (e.g.
> grepping for spaces/tabs, for LF, etc.) to verify the current rules
> match the files in the kernel tree?

Originally I sampled manually, but I have crafted a script to collect
more data. It's not 100% reliable, but good to get an idea. It reads
the leading whitespaces and if >80% of the lines have one kind of
indentation, it considers that it's the one used in that file. The
results, filtered to show only the relevant ones, are pasted at the
end.

These are some personal conclusions from the script's results:
- .py: although the official and most widely used style in the
community is 4-space indentation, in Linux tree many files use tabs.
What should we do here? 4-space is the clear standard for python...
- .rb: only one file in the whole tree
- .pm: only 3 files in the whole tree
- Files with many different indentations, better not to specify them:
rst, cocci, tc, xsl, manual pages
- Files that we should specify, tab indented: awk, dts, dtsi, dtso, s, S
- Files that we might specify, with preference for tab indenting but
not 100% clear: sh, bash, pl
- Files in tools/perf/scripts/*/bin/*: there is no clear formatting
for any file type, only for .py files that are tab-indented. To get
these results I've run my script from tools/perf/scripts directory.

>   - Would it be possible to go further than grepping and apply the
> rules (e.g. trigger a "save") through the entire tree to see whether
> there would be spurious changes?
>
>     If that comes out clean (or mostly clean), then we would be fairly
> confident this will not surprise developers (and it would be nice to
> have the script around for future updates of the `.editorconfig`).
>
>     Perhaps EditorConfig provides a script to check this already?
> Otherwise perhaps it can be done with editorconfig-core-c or
> editorconfig-vim or directly scripting on a couple editors?

It seems that EditorConfig libraries are only to parse the config
file, and applying the formatting is completely up to the linter or
editor's specific plugin.

I found a cli tool called editorconfig-checker that checks if there
are files that don't respect the formatting from .editorconfig, but it
gives tons of false positives.

Scripting any editor would be the way, but I don't have experience doing that.

>   - Are we sure the rules match the output of automated formatters we
> are using? (e.g. for Rust we enforce `rustfmt`, and thus we need to
> ensure the editor does not "fight" the formatter; otherwise developers
> may need to run the formatter more).

I'm only aware of Clang and Rust formatter configs in Linux tree, and
I think this complies with them. Do you know about any other?


SCRIPT FILTERED RESULTS:
Note: files might have "unknown" indentation for different reasons:
- there are different styles in a file
- the file contains only non-indented lines like simple Makefiles
- the script got confused with leading alignment whitespaces that
shouldn't be considered indentation
- others?

(1594 ignored files with unknown extension/shebang)

.0:
    tabs:    0
    2-space: 3
    3-space: 1
    4-space: 1
    6-space: 0
    8-space: 0
    unknown: 2

.1:
    tabs:    0
    2-space: 0
    3-space: 1
    4-space: 0
    6-space: 0
    8-space: 0
    unknown: 12

.2:
    tabs:    0
    2-space: 0
    3-space: 0
    4-space: 0
    6-space: 0
    8-space: 0
    unknown: 1

.8:
    tabs:    1
    2-space: 0
    3-space: 1
    4-space: 2
    6-space: 0
    8-space: 1
    unknown: 5

.S:
    tabs:    1253
    2-space: 8
    3-space: 7
    4-space: 2
    6-space: 0
    8-space: 13
    unknown: 47

.awk:
    tabs:    10
    2-space: 0
    3-space: 0
    4-space: 0
    6-space: 0
    8-space: 0
    unknown: 1

.awk (shebang):
    tabs:    2
    2-space: 0
    3-space: 0
    4-space: 0
    6-space: 0
    8-space: 0
    unknown: 0

.awk -f (shebang):
    tabs:    1
    2-space: 0
    3-space: 0
    4-space: 0
    6-space: 0
    8-space: 0
    unknown: 0

.bash (shebang):
    tabs:    12
    2-space: 0
    3-space: 0
    4-space: 5
    6-space: 0
    8-space: 1
    unknown: 38

.bconf:
    tabs:    7
    2-space: 3
    3-space: 0
    4-space: 0
    6-space: 0
    8-space: 0
    unknown: 19

.c:
    tabs:    35746
    2-space: 10
    3-space: 2
    4-space: 26
    6-space: 0
    8-space: 1
    unknown: 507

.cocci:
    tabs:    7
    2-space: 23
    3-space: 3
    4-space: 4
    6-space: 0
    8-space: 0
    unknown: 35

.dts:
    tabs:    2656
    2-space: 0
    3-space: 0
    4-space: 0
    6-space: 0
    8-space: 0
    unknown: 24

.dtsi:
    tabs:    1977
    2-space: 1
    3-space: 0
    4-space: 0
    6-space: 0
    8-space: 0
    unknown: 20

.dtso:
    tabs:    56
    2-space: 0
    3-space: 0
    4-space: 0
    6-space: 0
    8-space: 0
    unknown: 1

.h:
    tabs:    17546
    2-space: 93
    3-space: 65
    4-space: 220
    6-space: 2
    8-space: 40
    unknown: 6653

.json:
    tabs:    0
    2-space: 1
    3-space: 0
    4-space: 5
    6-space: 0
    8-space: 0
    unknown: 0

.pl:
    tabs:    28
    2-space: 1
    3-space: 0
    4-space: 2
    6-space: 0
    8-space: 0
    unknown: 18

.py:
    tabs:    43
    2-space: 14
    3-space: 0
    4-space: 87
    6-space: 0
    8-space: 2
    unknown: 6

.py (shebang):
    tabs:    5
    2-space: 0
    3-space: 0
    4-space: 5
    6-space: 0
    8-space: 1
    unknown: 0

.rb:
    tabs:    0
    2-space: 1
    3-space: 0
    4-space: 0
    6-space: 0
    8-space: 0
    unknown: 0

.rs:
    tabs:    0
    2-space: 0
    3-space: 0
    4-space: 35
    6-space: 0
    8-space: 0
    unknown: 3

.rst:
    tabs:    381
    2-space: 581
    3-space: 392
    4-space: 245
    6-space: 17
    8-space: 35
    unknown: 1670

.s:
    tabs:    4
    2-space: 0
    3-space: 0
    4-space: 0
    6-space: 0
    8-space: 0
    unknown: 0

.sh:
    tabs:    611
    2-space: 24
    3-space: 0
    4-space: 31
    6-space: 1
    8-space: 1
    unknown: 103

.sh (shebang):
    tabs:    26
    2-space: 4
    3-space: 0
    4-space: 2
    6-space: 0
    8-space: 0
    unknown: 14

.sh -x (shebang):
    tabs:    1
    2-space: 0
    3-space: 0
    4-space: 0
    6-space: 0
    8-space: 0
    unknown: 0

.tc:
    tabs:    8
    2-space: 20
    3-space: 0
    4-space: 40
    6-space: 0
    8-space: 0
    unknown: 34

.txt:
    tabs:    1045
    2-space: 103
    3-space: 8
    4-space: 57
    6-space: 2
    8-space: 14
    unknown: 686

.xsl:
    tabs:    2
    2-space: 0
    3-space: 0
    4-space: 0
    6-space: 0
    8-space: 0
    unknown: 8

.yaml:
    tabs:    0
    2-space: 3279
    3-space: 2
    4-space: 12
    6-space: 2
    8-space: 3
    unknown: 57

Kconfig:
    tabs:    1581
    2-space: 1
    3-space: 0
    4-space: 0
    6-space: 0
    8-space: 1
    unknown: 56

Makefile:
    tabs:    823
    2-space: 17
    3-space: 6
    4-space: 3
    6-space: 2
    8-space: 5
    unknown: 2005


>
> Cc'ing Andrew since he applied originally the `.clang-format`.
>
> Cheers,
> Miguel
>


-- 
Íñigo Huguet


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v3] Add .editorconfig file for basic formatting
  2023-04-14 10:10 [PATCH v3] Add .editorconfig file for basic formatting Íñigo Huguet
  2023-04-14 13:36 ` Miguel Ojeda
@ 2023-05-16 13:04 ` Vincent Mailhol
  2023-05-18  7:53   ` Íñigo Huguet
  1 sibling, 1 reply; 10+ messages in thread
From: Vincent Mailhol @ 2023-05-16 13:04 UTC (permalink / raw)
  To: ihuguet
  Cc: corbet, danny, jgg, joe, linux-kernel, linux, masahiroy, mic,
	ojeda, willy

Hi Íñigo,

Thank you very much for this patch. I would really love to see .editorconfig
added to the Linux tree.

I need to work on different project and so, since last year, I applied the v2 of
this series to my local tree and it works great.

On Fri, Apr 14, 2023 at 12:11 PM Íñigo Huguet <ihuguet@redhat.com> wrote:
> EditorConfig is a specification to define the most basic code formatting
> stuff, and it's supported by many editors and IDEs, either directly or
> via plugins, including VSCode/VSCodium, Vim, emacs and more.
> 
> It allows to define formatting style related to indentation, charset,
> end of lines and trailing whitespaces. It also allows to apply different
> formats for different files based on wildcards, so for example it is
> possible to apply different configs to *.{c,h}, *.py and *.rs.
> 
> In linux project, defining a .editorconfig might help to those people
> that work on different projects with different indentation styles, so
> they cannot define a global style. Now they will directly see the
> correct indentation on every fresh clone of the project.
> 
> See https://editorconfig.org
> 
> Link: https://lore.kernel.org/lkml/20200703073143.423557-1-danny@kdrag0n.dev/
> Link: https://lore.kernel.org/lkml/20230404075540.14422-1-ihuguet@redhat.com/
> Co-developed-by: Danny Lin <danny@kdrag0n.dev>
> Signed-off-by: Danny Lin <danny@kdrag0n.dev>
> Signed-off-by: Íñigo Huguet <ihuguet@redhat.com>
> ---
> v2:
>  - added special rule for patch files so it doesn't remove
>    trailing whitespaces, making them unusable.
> v3:
>  - moved all rules from [*] section to all the individual
>    sections so they doesn't affect to unexpected files.

I understand from from the past discussions that trim_trailing_whitespace or the
default indentation can not be apply broadly to all files. But what about those
three parameters?

  [*]
  charset = utf-8
  end_of_line = lf
  insert_final_newline = true

Those looks safe to me. Are there files for which we do not want utf-8 or for
which we do not what a final empty newline?

>  - added some extensions and files from a patch from Danny
>    Lin that didn't get to be merged:
>    https://lore.kernel.org/lkml/20200703073143.423557-1-danny@kdrag0n.dev/
>    However, the following file types hasn't been added
>    because they don't have a clear common style:
>    rst,pl,cocci,tc,bconf,svg,xsl,manual pages
> ---
>  .editorconfig                          | 30 ++++++++++++++++++++++++++
>  .gitignore                             |  1 +
>  Documentation/process/4.Coding.rst     |  4 ++++
>  Documentation/process/coding-style.rst |  4 ++++
>  4 files changed, 39 insertions(+)
>  create mode 100644 .editorconfig
> 
> diff --git a/.editorconfig b/.editorconfig
> new file mode 100644
> index 000000000000..dce20d45c246
> --- /dev/null
> +++ b/.editorconfig
> @@ -0,0 +1,30 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +
> +root = true
> +
> +# 8 width tabs
> +[{*.{c,h},Kconfig,Makefile,Makefile.*,*.mk}]
> +charset = utf-8
> +end_of_line = lf
> +trim_trailing_whitespace = true
> +insert_final_newline = true
> +indent_style = tab
> +indent_size = 8
> +
> +# 4 spaces
> +[{*.{json,pm,py,rs},tools/perf/scripts/*/bin/*}]
> +charset = utf-8
> +end_of_line = lf
> +trim_trailing_whitespace = true
> +insert_final_newline = true
> +indent_style = space
> +indent_size = 4
> +
> +# 2 spaces
> +[{*.{rb,yaml},.clang-format}]
> +charset = utf-8
> +end_of_line = lf
> +trim_trailing_whitespace = true
> +insert_final_newline = true
> +indent_style = space
> +indent_size = 2
> diff --git a/.gitignore b/.gitignore
> index 70ec6037fa7a..e4b3fe1d029b 100644
> --- a/.gitignore
> +++ b/.gitignore
> @@ -100,6 +100,7 @@ modules.order
>  #
>  !.clang-format
>  !.cocciconfig
> +!.editorconfig
>  !.get_maintainer.ignore
>  !.gitattributes
>  !.gitignore
> diff --git a/Documentation/process/4.Coding.rst b/Documentation/process/4.Coding.rst
> index 1f0d81f44e14..c2046dec0c2f 100644
> --- a/Documentation/process/4.Coding.rst
> +++ b/Documentation/process/4.Coding.rst
> @@ -66,6 +66,10 @@ for aligning variables/macros, for reflowing text and other similar tasks.
>  See the file :ref:`Documentation/process/clang-format.rst <clangformat>`
>  for more details.
>  
> +Some basic editor settings, such as indentation and line endings, will be
> +set automatically if you are using an editor that is compatible with
> +EditorConfig. See the official EditorConfig website for more information:
> +https://editorconfig.org/
>  
>  Abstraction layers
>  ******************
> diff --git a/Documentation/process/coding-style.rst b/Documentation/process/coding-style.rst
> index 007e49ef6cec..ec96462fa8be 100644
> --- a/Documentation/process/coding-style.rst
> +++ b/Documentation/process/coding-style.rst
> @@ -735,6 +735,10 @@ for aligning variables/macros, for reflowing text and other similar tasks.
>  See the file :ref:`Documentation/process/clang-format.rst <clangformat>`
>  for more details.
>  
> +Some basic editor settings, such as indentation and line endings, will be
> +set automatically if you are using an editor that is compatible with
> +EditorConfig. See the official EditorConfig website for more information:
> +https://editorconfig.org/
>  
>  10) Kconfig configuration files
>  -------------------------------
> -- 
> 2.39.2

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v3] Add .editorconfig file for basic formatting
  2023-05-16 13:04 ` Vincent Mailhol
@ 2023-05-18  7:53   ` Íñigo Huguet
  2023-05-18  8:59     ` Vincent MAILHOL
  0 siblings, 1 reply; 10+ messages in thread
From: Íñigo Huguet @ 2023-05-18  7:53 UTC (permalink / raw)
  To: Vincent Mailhol
  Cc: corbet, danny, jgg, joe, linux-kernel, linux, masahiroy, mic,
	ojeda, willy

Hi Vincent,

On Tue, May 16, 2023 at 3:05 PM Vincent Mailhol
<mailhol.vincent@wanadoo.fr> wrote:
>
> Hi Íñigo,
>
> Thank you very much for this patch. I would really love to see .editorconfig
> added to the Linux tree.
>
> I need to work on different project and so, since last year, I applied the v2 of
> this series to my local tree and it works great.
>
> On Fri, Apr 14, 2023 at 12:11 PM Íñigo Huguet <ihuguet@redhat.com> wrote:
> > EditorConfig is a specification to define the most basic code formatting
> > stuff, and it's supported by many editors and IDEs, either directly or
> > via plugins, including VSCode/VSCodium, Vim, emacs and more.
> >
> > It allows to define formatting style related to indentation, charset,
> > end of lines and trailing whitespaces. It also allows to apply different
> > formats for different files based on wildcards, so for example it is
> > possible to apply different configs to *.{c,h}, *.py and *.rs.
> >
> > In linux project, defining a .editorconfig might help to those people
> > that work on different projects with different indentation styles, so
> > they cannot define a global style. Now they will directly see the
> > correct indentation on every fresh clone of the project.
> >
> > See https://editorconfig.org
> >
> > Link: https://lore.kernel.org/lkml/20200703073143.423557-1-danny@kdrag0n.dev/
> > Link: https://lore.kernel.org/lkml/20230404075540.14422-1-ihuguet@redhat.com/
> > Co-developed-by: Danny Lin <danny@kdrag0n.dev>
> > Signed-off-by: Danny Lin <danny@kdrag0n.dev>
> > Signed-off-by: Íñigo Huguet <ihuguet@redhat.com>
> > ---
> > v2:
> >  - added special rule for patch files so it doesn't remove
> >    trailing whitespaces, making them unusable.
> > v3:
> >  - moved all rules from [*] section to all the individual
> >    sections so they doesn't affect to unexpected files.
>
> I understand from from the past discussions that trim_trailing_whitespace or the
> default indentation can not be apply broadly to all files. But what about those
> three parameters?
>
>   [*]
>   charset = utf-8
>   end_of_line = lf
>   insert_final_newline = true
>
> Those looks safe to me. Are there files for which we do not want utf-8 or for
> which we do not what a final empty newline?

Yes, I think that they are probably safe to use, but Miguel thought it
was better to be more cautious, and I agree. We can expand adding more
file formats when we detect those that are not covered.

With v3, the most used files are covered, and since there are
thousands of files with many different purposes, it's very difficult
to answer if there are files where we don't want these settings.

For example, if there are a few files that, who knows why, need a
different encoding, we can silently corrupt the file and cause a bad
debugging time for a developer. For the end of line and final newline,
we already saw that there are files where they are undesired, like
patch files. There might be more.

>
> >  - added some extensions and files from a patch from Danny
> >    Lin that didn't get to be merged:
> >    https://lore.kernel.org/lkml/20200703073143.423557-1-danny@kdrag0n.dev/
> >    However, the following file types hasn't been added
> >    because they don't have a clear common style:
> >    rst,pl,cocci,tc,bconf,svg,xsl,manual pages
> > ---
> >  .editorconfig                          | 30 ++++++++++++++++++++++++++
> >  .gitignore                             |  1 +
> >  Documentation/process/4.Coding.rst     |  4 ++++
> >  Documentation/process/coding-style.rst |  4 ++++
> >  4 files changed, 39 insertions(+)
> >  create mode 100644 .editorconfig
> >
> > diff --git a/.editorconfig b/.editorconfig
> > new file mode 100644
> > index 000000000000..dce20d45c246
> > --- /dev/null
> > +++ b/.editorconfig
> > @@ -0,0 +1,30 @@
> > +# SPDX-License-Identifier: GPL-2.0-only
> > +
> > +root = true
> > +
> > +# 8 width tabs
> > +[{*.{c,h},Kconfig,Makefile,Makefile.*,*.mk}]
> > +charset = utf-8
> > +end_of_line = lf
> > +trim_trailing_whitespace = true
> > +insert_final_newline = true
> > +indent_style = tab
> > +indent_size = 8
> > +
> > +# 4 spaces
> > +[{*.{json,pm,py,rs},tools/perf/scripts/*/bin/*}]
> > +charset = utf-8
> > +end_of_line = lf
> > +trim_trailing_whitespace = true
> > +insert_final_newline = true
> > +indent_style = space
> > +indent_size = 4
> > +
> > +# 2 spaces
> > +[{*.{rb,yaml},.clang-format}]
> > +charset = utf-8
> > +end_of_line = lf
> > +trim_trailing_whitespace = true
> > +insert_final_newline = true
> > +indent_style = space
> > +indent_size = 2
> > diff --git a/.gitignore b/.gitignore
> > index 70ec6037fa7a..e4b3fe1d029b 100644
> > --- a/.gitignore
> > +++ b/.gitignore
> > @@ -100,6 +100,7 @@ modules.order
> >  #
> >  !.clang-format
> >  !.cocciconfig
> > +!.editorconfig
> >  !.get_maintainer.ignore
> >  !.gitattributes
> >  !.gitignore
> > diff --git a/Documentation/process/4.Coding.rst b/Documentation/process/4.Coding.rst
> > index 1f0d81f44e14..c2046dec0c2f 100644
> > --- a/Documentation/process/4.Coding.rst
> > +++ b/Documentation/process/4.Coding.rst
> > @@ -66,6 +66,10 @@ for aligning variables/macros, for reflowing text and other similar tasks.
> >  See the file :ref:`Documentation/process/clang-format.rst <clangformat>`
> >  for more details.
> >
> > +Some basic editor settings, such as indentation and line endings, will be
> > +set automatically if you are using an editor that is compatible with
> > +EditorConfig. See the official EditorConfig website for more information:
> > +https://editorconfig.org/
> >
> >  Abstraction layers
> >  ******************
> > diff --git a/Documentation/process/coding-style.rst b/Documentation/process/coding-style.rst
> > index 007e49ef6cec..ec96462fa8be 100644
> > --- a/Documentation/process/coding-style.rst
> > +++ b/Documentation/process/coding-style.rst
> > @@ -735,6 +735,10 @@ for aligning variables/macros, for reflowing text and other similar tasks.
> >  See the file :ref:`Documentation/process/clang-format.rst <clangformat>`
> >  for more details.
> >
> > +Some basic editor settings, such as indentation and line endings, will be
> > +set automatically if you are using an editor that is compatible with
> > +EditorConfig. See the official EditorConfig website for more information:
> > +https://editorconfig.org/
> >
> >  10) Kconfig configuration files
> >  -------------------------------
> > --
> > 2.39.2
>


-- 
Íñigo Huguet


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v3] Add .editorconfig file for basic formatting
  2023-05-18  7:53   ` Íñigo Huguet
@ 2023-05-18  8:59     ` Vincent MAILHOL
  2023-05-18  9:16       ` Íñigo Huguet
  0 siblings, 1 reply; 10+ messages in thread
From: Vincent MAILHOL @ 2023-05-18  8:59 UTC (permalink / raw)
  To: Íñigo Huguet
  Cc: corbet, danny, jgg, joe, linux-kernel, linux, masahiroy, mic,
	ojeda, willy

On Thu. 18 May 2023 at 16:53, Íñigo Huguet <ihuguet@redhat.com> wrote:
> Hi Vincent,
>
> On Tue, May 16, 2023 at 3:05 PM Vincent Mailhol
> <mailhol.vincent@wanadoo.fr> wrote:
> >
> > Hi Íñigo,
> >
> > Thank you very much for this patch. I would really love to see .editorconfig
> > added to the Linux tree.
> >
> > I need to work on different project and so, since last year, I applied the v2 of
> > this series to my local tree and it works great.
> >
> > On Fri, Apr 14, 2023 at 12:11 PM Íñigo Huguet <ihuguet@redhat.com> wrote:
> > > EditorConfig is a specification to define the most basic code formatting
> > > stuff, and it's supported by many editors and IDEs, either directly or
> > > via plugins, including VSCode/VSCodium, Vim, emacs and more.
> > >
> > > It allows to define formatting style related to indentation, charset,
> > > end of lines and trailing whitespaces. It also allows to apply different
> > > formats for different files based on wildcards, so for example it is
> > > possible to apply different configs to *.{c,h}, *.py and *.rs.
> > >
> > > In linux project, defining a .editorconfig might help to those people
> > > that work on different projects with different indentation styles, so
> > > they cannot define a global style. Now they will directly see the
> > > correct indentation on every fresh clone of the project.
> > >
> > > See https://editorconfig.org
> > >
> > > Link: https://lore.kernel.org/lkml/20200703073143.423557-1-danny@kdrag0n.dev/
> > > Link: https://lore.kernel.org/lkml/20230404075540.14422-1-ihuguet@redhat.com/
> > > Co-developed-by: Danny Lin <danny@kdrag0n.dev>
> > > Signed-off-by: Danny Lin <danny@kdrag0n.dev>
> > > Signed-off-by: Íñigo Huguet <ihuguet@redhat.com>
> > > ---
> > > v2:
> > >  - added special rule for patch files so it doesn't remove
> > >    trailing whitespaces, making them unusable.
> > > v3:
> > >  - moved all rules from [*] section to all the individual
> > >    sections so they doesn't affect to unexpected files.
> >
> > I understand from from the past discussions that trim_trailing_whitespace or the
> > default indentation can not be apply broadly to all files. But what about those
> > three parameters?
> >
> >   [*]
> >   charset = utf-8
> >   end_of_line = lf
> >   insert_final_newline = true
> >
> > Those looks safe to me. Are there files for which we do not want utf-8 or for
> > which we do not what a final empty newline?
>
> Yes, I think that they are probably safe to use, but Miguel thought it
> was better to be more cautious, and I agree. We can expand adding more
> file formats when we detect those that are not covered.

I think you are referring to this message from Miguel:

  While UTF-8 and LF are probably OK for all files, I am not 100% sure about:

  +insert_final_newline = true
  +indent_style = tab
  +indent_size = 8

Link: https://lore.kernel.org/lkml/CANiq72k2rrByxzj1c4azAVJq-V7BqQcmBwtm3XM9T8r3r3-ysQ@mail.gmail.com/

So it seems that we all agree on the UTF-8 and LF. Or did I miss
another message?

Regardless, with or without my nitpick addressed, it looks good to me:

Reviewed-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
Tested-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr>

> With v3, the most used files are covered, and since there are
> thousands of files with many different purposes, it's very difficult
> to answer if there are files where we don't want these settings.
>
> For example, if there are a few files that, who knows why, need a
> different encoding, we can silently corrupt the file and cause a bad
> debugging time for a developer. For the end of line and final newline,
> we already saw that there are files where they are undesired, like
> patch files. There might be more.
>
> >
> > >  - added some extensions and files from a patch from Danny
> > >    Lin that didn't get to be merged:
> > >    https://lore.kernel.org/lkml/20200703073143.423557-1-danny@kdrag0n.dev/
> > >    However, the following file types hasn't been added
> > >    because they don't have a clear common style:
> > >    rst,pl,cocci,tc,bconf,svg,xsl,manual pages
> > > ---
> > >  .editorconfig                          | 30 ++++++++++++++++++++++++++
> > >  .gitignore                             |  1 +
> > >  Documentation/process/4.Coding.rst     |  4 ++++
> > >  Documentation/process/coding-style.rst |  4 ++++
> > >  4 files changed, 39 insertions(+)
> > >  create mode 100644 .editorconfig
> > >
> > > diff --git a/.editorconfig b/.editorconfig
> > > new file mode 100644
> > > index 000000000000..dce20d45c246
> > > --- /dev/null
> > > +++ b/.editorconfig
> > > @@ -0,0 +1,30 @@
> > > +# SPDX-License-Identifier: GPL-2.0-only
> > > +
> > > +root = true
> > > +
> > > +# 8 width tabs
> > > +[{*.{c,h},Kconfig,Makefile,Makefile.*,*.mk}]
> > > +charset = utf-8
> > > +end_of_line = lf
> > > +trim_trailing_whitespace = true
> > > +insert_final_newline = true
> > > +indent_style = tab
> > > +indent_size = 8
> > > +
> > > +# 4 spaces
> > > +[{*.{json,pm,py,rs},tools/perf/scripts/*/bin/*}]
> > > +charset = utf-8
> > > +end_of_line = lf
> > > +trim_trailing_whitespace = true
> > > +insert_final_newline = true
> > > +indent_style = space
> > > +indent_size = 4
> > > +
> > > +# 2 spaces
> > > +[{*.{rb,yaml},.clang-format}]
> > > +charset = utf-8
> > > +end_of_line = lf
> > > +trim_trailing_whitespace = true
> > > +insert_final_newline = true
> > > +indent_style = space
> > > +indent_size = 2
> > > diff --git a/.gitignore b/.gitignore
> > > index 70ec6037fa7a..e4b3fe1d029b 100644
> > > --- a/.gitignore
> > > +++ b/.gitignore
> > > @@ -100,6 +100,7 @@ modules.order
> > >  #
> > >  !.clang-format
> > >  !.cocciconfig
> > > +!.editorconfig
> > >  !.get_maintainer.ignore
> > >  !.gitattributes
> > >  !.gitignore
> > > diff --git a/Documentation/process/4.Coding.rst b/Documentation/process/4.Coding.rst
> > > index 1f0d81f44e14..c2046dec0c2f 100644
> > > --- a/Documentation/process/4.Coding.rst
> > > +++ b/Documentation/process/4.Coding.rst
> > > @@ -66,6 +66,10 @@ for aligning variables/macros, for reflowing text and other similar tasks.
> > >  See the file :ref:`Documentation/process/clang-format.rst <clangformat>`
> > >  for more details.
> > >
> > > +Some basic editor settings, such as indentation and line endings, will be
> > > +set automatically if you are using an editor that is compatible with
> > > +EditorConfig. See the official EditorConfig website for more information:
> > > +https://editorconfig.org/
> > >
> > >  Abstraction layers
> > >  ******************
> > > diff --git a/Documentation/process/coding-style.rst b/Documentation/process/coding-style.rst
> > > index 007e49ef6cec..ec96462fa8be 100644
> > > --- a/Documentation/process/coding-style.rst
> > > +++ b/Documentation/process/coding-style.rst
> > > @@ -735,6 +735,10 @@ for aligning variables/macros, for reflowing text and other similar tasks.
> > >  See the file :ref:`Documentation/process/clang-format.rst <clangformat>`
> > >  for more details.
> > >
> > > +Some basic editor settings, such as indentation and line endings, will be
> > > +set automatically if you are using an editor that is compatible with
> > > +EditorConfig. See the official EditorConfig website for more information:
> > > +https://editorconfig.org/
> > >
> > >  10) Kconfig configuration files
> > >  -------------------------------
> > > --

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v3] Add .editorconfig file for basic formatting
  2023-05-18  8:59     ` Vincent MAILHOL
@ 2023-05-18  9:16       ` Íñigo Huguet
  0 siblings, 0 replies; 10+ messages in thread
From: Íñigo Huguet @ 2023-05-18  9:16 UTC (permalink / raw)
  To: Vincent MAILHOL
  Cc: corbet, danny, jgg, joe, linux-kernel, linux, masahiroy, mic,
	ojeda, willy

On Thu, May 18, 2023 at 10:59 AM Vincent MAILHOL
<mailhol.vincent@wanadoo.fr> wrote:
>
> On Thu. 18 May 2023 at 16:53, Íñigo Huguet <ihuguet@redhat.com> wrote:
> > Hi Vincent,
> >
> > On Tue, May 16, 2023 at 3:05 PM Vincent Mailhol
> > <mailhol.vincent@wanadoo.fr> wrote:
> > >
> > > Hi Íñigo,
> > >
> > > Thank you very much for this patch. I would really love to see .editorconfig
> > > added to the Linux tree.
> > >
> > > I need to work on different project and so, since last year, I applied the v2 of
> > > this series to my local tree and it works great.
> > >
> > > On Fri, Apr 14, 2023 at 12:11 PM Íñigo Huguet <ihuguet@redhat.com> wrote:
> > > > EditorConfig is a specification to define the most basic code formatting
> > > > stuff, and it's supported by many editors and IDEs, either directly or
> > > > via plugins, including VSCode/VSCodium, Vim, emacs and more.
> > > >
> > > > It allows to define formatting style related to indentation, charset,
> > > > end of lines and trailing whitespaces. It also allows to apply different
> > > > formats for different files based on wildcards, so for example it is
> > > > possible to apply different configs to *.{c,h}, *.py and *.rs.
> > > >
> > > > In linux project, defining a .editorconfig might help to those people
> > > > that work on different projects with different indentation styles, so
> > > > they cannot define a global style. Now they will directly see the
> > > > correct indentation on every fresh clone of the project.
> > > >
> > > > See https://editorconfig.org
> > > >
> > > > Link: https://lore.kernel.org/lkml/20200703073143.423557-1-danny@kdrag0n.dev/
> > > > Link: https://lore.kernel.org/lkml/20230404075540.14422-1-ihuguet@redhat.com/
> > > > Co-developed-by: Danny Lin <danny@kdrag0n.dev>
> > > > Signed-off-by: Danny Lin <danny@kdrag0n.dev>
> > > > Signed-off-by: Íñigo Huguet <ihuguet@redhat.com>
> > > > ---
> > > > v2:
> > > >  - added special rule for patch files so it doesn't remove
> > > >    trailing whitespaces, making them unusable.
> > > > v3:
> > > >  - moved all rules from [*] section to all the individual
> > > >    sections so they doesn't affect to unexpected files.
> > >
> > > I understand from from the past discussions that trim_trailing_whitespace or the
> > > default indentation can not be apply broadly to all files. But what about those
> > > three parameters?
> > >
> > >   [*]
> > >   charset = utf-8
> > >   end_of_line = lf
> > >   insert_final_newline = true
> > >
> > > Those looks safe to me. Are there files for which we do not want utf-8 or for
> > > which we do not what a final empty newline?
> >
> > Yes, I think that they are probably safe to use, but Miguel thought it
> > was better to be more cautious, and I agree. We can expand adding more
> > file formats when we detect those that are not covered.
>
> I think you are referring to this message from Miguel:
>
>   While UTF-8 and LF are probably OK for all files, I am not 100% sure about:
>
>   +insert_final_newline = true
>   +indent_style = tab
>   +indent_size = 8
>
> Link: https://lore.kernel.org/lkml/CANiq72k2rrByxzj1c4azAVJq-V7BqQcmBwtm3XM9T8r3r3-ysQ@mail.gmail.com/
>
> So it seems that we all agree on the UTF-8 and LF. Or did I miss
> another message?

It seems so. The patch you link is the original path sent by Danny
months ago. This is v3 of my own patch, that I sent without knowing
that Danny's one existed:
https://lore.kernel.org/lkml/20230404075540.14422-1-ihuguet@redhat.com/

Although there were no explicit complains about UTF-8 and LF, I feel
that it's safer to not add a [*] section at all.

>
> Regardless, with or without my nitpick addressed, it looks good to me:
>
> Reviewed-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr>
> Tested-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr>

I was going to prepare a v4 with small modifications, at the light of
the results I posted here:
https://lore.kernel.org/lkml/CACT4ouf2M1k7SaMgqv1Fj33Wen7UKuUyKp-Y9oer+THiWEebNg@mail.gmail.com/

I was waiting for some feedback about them, but no responses received
so far so I will go ahead making the changes with my own criteria.

>
> > With v3, the most used files are covered, and since there are
> > thousands of files with many different purposes, it's very difficult
> > to answer if there are files where we don't want these settings.
> >
> > For example, if there are a few files that, who knows why, need a
> > different encoding, we can silently corrupt the file and cause a bad
> > debugging time for a developer. For the end of line and final newline,
> > we already saw that there are files where they are undesired, like
> > patch files. There might be more.
> >
> > >
> > > >  - added some extensions and files from a patch from Danny
> > > >    Lin that didn't get to be merged:
> > > >    https://lore.kernel.org/lkml/20200703073143.423557-1-danny@kdrag0n.dev/
> > > >    However, the following file types hasn't been added
> > > >    because they don't have a clear common style:
> > > >    rst,pl,cocci,tc,bconf,svg,xsl,manual pages
> > > > ---
> > > >  .editorconfig                          | 30 ++++++++++++++++++++++++++
> > > >  .gitignore                             |  1 +
> > > >  Documentation/process/4.Coding.rst     |  4 ++++
> > > >  Documentation/process/coding-style.rst |  4 ++++
> > > >  4 files changed, 39 insertions(+)
> > > >  create mode 100644 .editorconfig
> > > >
> > > > diff --git a/.editorconfig b/.editorconfig
> > > > new file mode 100644
> > > > index 000000000000..dce20d45c246
> > > > --- /dev/null
> > > > +++ b/.editorconfig
> > > > @@ -0,0 +1,30 @@
> > > > +# SPDX-License-Identifier: GPL-2.0-only
> > > > +
> > > > +root = true
> > > > +
> > > > +# 8 width tabs
> > > > +[{*.{c,h},Kconfig,Makefile,Makefile.*,*.mk}]
> > > > +charset = utf-8
> > > > +end_of_line = lf
> > > > +trim_trailing_whitespace = true
> > > > +insert_final_newline = true
> > > > +indent_style = tab
> > > > +indent_size = 8
> > > > +
> > > > +# 4 spaces
> > > > +[{*.{json,pm,py,rs},tools/perf/scripts/*/bin/*}]
> > > > +charset = utf-8
> > > > +end_of_line = lf
> > > > +trim_trailing_whitespace = true
> > > > +insert_final_newline = true
> > > > +indent_style = space
> > > > +indent_size = 4
> > > > +
> > > > +# 2 spaces
> > > > +[{*.{rb,yaml},.clang-format}]
> > > > +charset = utf-8
> > > > +end_of_line = lf
> > > > +trim_trailing_whitespace = true
> > > > +insert_final_newline = true
> > > > +indent_style = space
> > > > +indent_size = 2
> > > > diff --git a/.gitignore b/.gitignore
> > > > index 70ec6037fa7a..e4b3fe1d029b 100644
> > > > --- a/.gitignore
> > > > +++ b/.gitignore
> > > > @@ -100,6 +100,7 @@ modules.order
> > > >  #
> > > >  !.clang-format
> > > >  !.cocciconfig
> > > > +!.editorconfig
> > > >  !.get_maintainer.ignore
> > > >  !.gitattributes
> > > >  !.gitignore
> > > > diff --git a/Documentation/process/4.Coding.rst b/Documentation/process/4.Coding.rst
> > > > index 1f0d81f44e14..c2046dec0c2f 100644
> > > > --- a/Documentation/process/4.Coding.rst
> > > > +++ b/Documentation/process/4.Coding.rst
> > > > @@ -66,6 +66,10 @@ for aligning variables/macros, for reflowing text and other similar tasks.
> > > >  See the file :ref:`Documentation/process/clang-format.rst <clangformat>`
> > > >  for more details.
> > > >
> > > > +Some basic editor settings, such as indentation and line endings, will be
> > > > +set automatically if you are using an editor that is compatible with
> > > > +EditorConfig. See the official EditorConfig website for more information:
> > > > +https://editorconfig.org/
> > > >
> > > >  Abstraction layers
> > > >  ******************
> > > > diff --git a/Documentation/process/coding-style.rst b/Documentation/process/coding-style.rst
> > > > index 007e49ef6cec..ec96462fa8be 100644
> > > > --- a/Documentation/process/coding-style.rst
> > > > +++ b/Documentation/process/coding-style.rst
> > > > @@ -735,6 +735,10 @@ for aligning variables/macros, for reflowing text and other similar tasks.
> > > >  See the file :ref:`Documentation/process/clang-format.rst <clangformat>`
> > > >  for more details.
> > > >
> > > > +Some basic editor settings, such as indentation and line endings, will be
> > > > +set automatically if you are using an editor that is compatible with
> > > > +EditorConfig. See the official EditorConfig website for more information:
> > > > +https://editorconfig.org/
> > > >
> > > >  10) Kconfig configuration files
> > > >  -------------------------------
> > > > --
>


-- 
Íñigo Huguet


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v3] Add .editorconfig file for basic formatting
  2023-05-08  8:58   ` Íñigo Huguet
@ 2023-05-26 21:16     ` Miguel Ojeda
  2023-05-30  7:52       ` Íñigo Huguet
  0 siblings, 1 reply; 10+ messages in thread
From: Miguel Ojeda @ 2023-05-26 21:16 UTC (permalink / raw)
  To: Íñigo Huguet
  Cc: ojeda, masahiroy, jgg, mic, danny, linux-kernel, corbet, joe,
	linux, willy, Andrew Morton

On Mon, May 8, 2023 at 10:59 AM Íñigo Huguet <ihuguet@redhat.com> wrote:
>
> Originally I sampled manually, but I have crafted a script to collect
> more data. It's not 100% reliable, but good to get an idea. It reads
> the leading whitespaces and if >80% of the lines have one kind of
> indentation, it considers that it's the one used in that file. The
> results, filtered to show only the relevant ones, are pasted at the
> end.

This is useful -- thanks a lot for working on collecting it!

> These are some personal conclusions from the script's results:
> - .py: although the official and most widely used style in the
> community is 4-space indentation, in Linux tree many files use tabs.
> What should we do here? 4-space is the clear standard for python...

Yeah, this is the kind of thing that worries me and why I asked --
what do editors do when they have the config saying it is 4-spaces,
but the file is tabs? Do they adjust, do they convert the entire file,
or do they simply start mixing indentation styles? Does the
`.editorconfig` spec say anything about it? For instance, here is an
issue about this sort of problem:

    https://github.com/editorconfig/editorconfig-vscode/issues/329

If the rule could be applied only to new files, then it would be
fairly easy to decide, given the majority uses 4-spaces and it nicely
aligns with PEP 8, Black, etc. But unless we are quite sure we are not
annoying developers, I would avoid specifying anything in these cases.

In some cases (e.g. few files), you may be able to propose to
normalize the indentation style treewide for that extension.

> - .rb: only one file in the whole tree
> - .pm: only 3 files in the whole tree

I guess you could also ignore extensions without many matches in order
to simplify -- they can always be added later, ideally by their
maintainers.

> - Files with many different indentations, better not to specify them:
> rst, cocci, tc, xsl, manual pages
> - Files that we should specify, tab indented: awk, dts, dtsi, dtso, s, S
> - Files that we might specify, with preference for tab indenting but
> not 100% clear: sh, bash, pl
> - Files in tools/perf/scripts/*/bin/*: there is no clear formatting
> for any file type, only for .py files that are tab-indented. To get
> these results I've run my script from tools/perf/scripts directory.

If all Python tab-indented files are in a given folder, then would it
be possible to provide an `.editorconfig` for that folder, and then
4-spaces for the global one? i.e. splitting the problem across folders
may be a solution (within reason, of course, i.e. as long as we don't
fill the kernel with `.editorconfig` files... :)

> I'm only aware of Clang and Rust formatter configs in Linux tree, and
> I think this complies with them. Do you know about any other?

There is `scripts/checkpatch.pl`, which I guess may be counted as one
since one can fix what it complains about manually (and I think it has
some "fix in place" support too).

There is also `Documentation/devicetree/bindings/.yamllint`.

In addition, some may be using formatters in a default config? e.g.
Black for some of the Python scripts.

Cheers,
Miguel

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v3] Add .editorconfig file for basic formatting
  2023-05-26 21:16     ` Miguel Ojeda
@ 2023-05-30  7:52       ` Íñigo Huguet
  2023-05-30 12:52         ` Joe Perches
  0 siblings, 1 reply; 10+ messages in thread
From: Íñigo Huguet @ 2023-05-30  7:52 UTC (permalink / raw)
  To: Miguel Ojeda
  Cc: ojeda, masahiroy, jgg, mic, danny, linux-kernel, corbet, joe,
	linux, willy, Andrew Morton

On Fri, May 26, 2023 at 11:16 PM Miguel Ojeda
<miguel.ojeda.sandonis@gmail.com> wrote:
>
> On Mon, May 8, 2023 at 10:59 AM Íñigo Huguet <ihuguet@redhat.com> wrote:
> >
> > Originally I sampled manually, but I have crafted a script to collect
> > more data. It's not 100% reliable, but good to get an idea. It reads
> > the leading whitespaces and if >80% of the lines have one kind of
> > indentation, it considers that it's the one used in that file. The
> > results, filtered to show only the relevant ones, are pasted at the
> > end.
>
> This is useful -- thanks a lot for working on collecting it!
>
> > These are some personal conclusions from the script's results:
> > - .py: although the official and most widely used style in the
> > community is 4-space indentation, in Linux tree many files use tabs.
> > What should we do here? 4-space is the clear standard for python...
>
> Yeah, this is the kind of thing that worries me and why I asked --
> what do editors do when they have the config saying it is 4-spaces,
> but the file is tabs? Do they adjust, do they convert the entire file,
> or do they simply start mixing indentation styles? Does the
> `.editorconfig` spec say anything about it? For instance, here is an
> issue about this sort of problem:
>
>     https://github.com/editorconfig/editorconfig-vscode/issues/329
>
> If the rule could be applied only to new files, then it would be
> fairly easy to decide, given the majority uses 4-spaces and it nicely
> aligns with PEP 8, Black, etc. But unless we are quite sure we are not
> annoying developers, I would avoid specifying anything in these cases.

I don't think it's possible to apply only to new files, and if it were
possible, it would be up to each editor plugin, not editorconfig
"core".

What we can do is differentiate some directories, see below.

> In some cases (e.g. few files), you may be able to propose to
> normalize the indentation style treewide for that extension.
>
> > - .rb: only one file in the whole tree
> > - .pm: only 3 files in the whole tree
>
> I guess you could also ignore extensions without many matches in order
> to simplify -- they can always be added later, ideally by their
> maintainers.

Agree

> > - Files with many different indentations, better not to specify them:
> > rst, cocci, tc, xsl, manual pages
> > - Files that we should specify, tab indented: awk, dts, dtsi, dtso, s, S
> > - Files that we might specify, with preference for tab indenting but
> > not 100% clear: sh, bash, pl

Not sure what to do with these 3 (sh, bash, pl): a big majority are
tab indented, but there are some files with inconsistent indentations
within the same file (i.e. scripts/get_maintainer.pl). Honestly I
would be tempted of adding them too.

> > - Files in tools/perf/scripts/*/bin/*: there is no clear formatting
> > for any file type, only for .py files that are tab-indented. To get
> > these results I've run my script from tools/perf/scripts directory.
>
> If all Python tab-indented files are in a given folder, then would it
> be possible to provide an `.editorconfig` for that folder, and then
> 4-spaces for the global one? i.e. splitting the problem across folders
> may be a solution (within reason, of course, i.e. as long as we don't
> fill the kernel with `.editorconfig` files... :)

We don't need to create different .editorconfig files. A single
.editorconfig must be read from top to bottom, with the last setting
that matched taking precedence, per the spec.
At the end of this message there is the list of all python files in
the source tree with their indentation style. According to that, we
could use this config:

[*.py]
indent_style = space
indent_size = 4

[tools/{perf,power,rcu,testing/kunit}/**.py,]
indent_style = tab
indent_size = 8

> > I'm only aware of Clang and Rust formatter configs in Linux tree, and
> > I think this complies with them. Do you know about any other?
>
> There is `scripts/checkpatch.pl`, which I guess may be counted as one
> since one can fix what it complains about manually (and I think it has
> some "fix in place" support too).

Checkpatch has many rules, but they mostly affect to C files where we
have already set the official style, right?

> There is also `Documentation/devicetree/bindings/.yamllint`.

Indentation is 2 spaces so it's OK, but it has the rule
"trailing-spaces: false". According to yamllint docs: "Use this rule
to forbid trailing spaces at the end of lines". So, setting it to
false should mean that trailing spaces are ALLOWED, correct?

> In addition, some may be using formatters in a default config? e.g.
> Black for some of the Python scripts.

But this is not something we can really support. If someone is using 2
different formatters in the same project, conflicts are expected to
happen. They will need to disable one of them.


These are the type of indentation for each of the python files in the tree:
4spa: ./Documentation/conf.py
tabs: ./Documentation/networking/device_drivers/atm/cxacru-cf.py
2spa: ./Documentation/sphinx/automarkup.py
4spa: ./Documentation/sphinx/cdomain.py
4spa: ./Documentation/sphinx/kernel_abi.py
4spa: ./Documentation/sphinx/kernel_feat.py
4spa: ./Documentation/sphinx/kernel_include.py
4spa: ./Documentation/sphinx/kerneldoc.py
4spa: ./Documentation/sphinx/kernellog.py
4spa: ./Documentation/sphinx/kfigure.py
4spa: ./Documentation/sphinx/load_config.py
4spa: ./Documentation/sphinx/maintainers_include.py
4spa: ./Documentation/sphinx/rstFlatTable.py
tabs: ./Documentation/target/tcm_mod_builder.py
tabs: ./Documentation/trace/postprocess/decode_msr.py
4spa: ./Documentation/userspace-api/media/conf_nitpick.py
4spa: ./arch/ia64/scripts/unwcheck.py
2spa: ./drivers/comedi/drivers/ni_routing/tools/convert_csv_to_c.py
2spa: ./drivers/comedi/drivers/ni_routing/tools/convert_py_to_csv.py
2spa: ./drivers/comedi/drivers/ni_routing/tools/csv_collection.py
2spa: ./drivers/comedi/drivers/ni_routing/tools/make_blank_csv.py
2spa: ./drivers/comedi/drivers/ni_routing/tools/ni_names.py
tabs: ./drivers/staging/greybus/tools/lbtest
4spa: ./scripts/bloat-o-meter
4spa: ./scripts/bpf_doc.py
4spa: ./scripts/checkkconfigsymbols.py
4spa: ./scripts/clang-tools/gen_compile_commands.py
4spa: ./scripts/clang-tools/run-clang-tools.py
4spa: ./scripts/diffconfig
tabs: ./scripts/dtc/dt-extract-compatibles
unknown-0: ./scripts/gdb/linux/__init__.py
4spa: ./scripts/gdb/linux/clk.py
4spa: ./scripts/gdb/linux/config.py
4spa: ./scripts/gdb/linux/cpus.py
4spa: ./scripts/gdb/linux/device.py
4spa: ./scripts/gdb/linux/dmesg.py
4spa: ./scripts/gdb/linux/genpd.py
2spa: ./scripts/gdb/linux/lists.py
4spa: ./scripts/gdb/linux/mm.py
4spa: ./scripts/gdb/linux/modules.py
4spa: ./scripts/gdb/linux/proc.py
4spa: ./scripts/gdb/linux/rbtree.py
4spa: ./scripts/gdb/linux/symbols.py
4spa: ./scripts/gdb/linux/tasks.py
4spa: ./scripts/gdb/linux/timerlist.py
4spa: ./scripts/gdb/linux/utils.py
4spa: ./scripts/gdb/vmlinux-gdb.py
4spa: ./scripts/generate_rust_analyzer.py
tabs: ./scripts/jobserver-exec
4spa: ./scripts/kconfig/tests/auto_submenu/__init__.py
4spa: ./scripts/kconfig/tests/choice/__init__.py
4spa: ./scripts/kconfig/tests/choice_value_with_m_dep/__init__.py
4spa: ./scripts/kconfig/tests/conftest.py
4spa: ./scripts/kconfig/tests/err_recursive_dep/__init__.py
4spa: ./scripts/kconfig/tests/err_recursive_inc/__init__.py
4spa: ./scripts/kconfig/tests/inter_choice/__init__.py
4spa: ./scripts/kconfig/tests/new_choice_with_dep/__init__.py
4spa: ./scripts/kconfig/tests/no_write_if_dep_unmet/__init__.py
4spa: ./scripts/kconfig/tests/preprocess/builtin_func/__init__.py
4spa: ./scripts/kconfig/tests/preprocess/circular_expansion/__init__.py
4spa: ./scripts/kconfig/tests/preprocess/escape/__init__.py
4spa: ./scripts/kconfig/tests/preprocess/variable/__init__.py
tabs: ./scripts/show_delta
4spa: ./scripts/spdxcheck.py
tabs: ./scripts/tracing/draw_functrace.py
4spa: ./tools/cgroup/iocost_coef_gen.py
4spa: ./tools/cgroup/iocost_monitor.py
4spa: ./tools/cgroup/memcg_shrinker.py
4spa: ./tools/cgroup/memcg_slabinfo.py
tabs: ./tools/hv/lsvmbus
8spa: ./tools/hv/vmbus_testing
4spa: ./tools/kvm/kvm_stat/kvm_stat
4spa: ./tools/net/ynl/cli.py
unknown-mix: ./tools/net/ynl/lib/__init__.py
4spa: ./tools/net/ynl/lib/nlspec.py
4spa: ./tools/net/ynl/lib/ynl.py
4spa: ./tools/net/ynl/ynl-gen-c.py
2spa: ./tools/perf/pmu-events/jevents.py
2spa: ./tools/perf/pmu-events/metric.py
4spa: ./tools/perf/pmu-events/metric_test.py
unknown-mix: ./tools/perf/python/tracepoint.py
tabs: ./tools/perf/python/twatch.py
unknown-mix: ./tools/perf/scripts/python/Perf-Trace-Util/lib/Perf/Trace/Core.py
8spa: ./tools/perf/scripts/python/Perf-Trace-Util/lib/Perf/Trace/EventClass.py
tabs: ./tools/perf/scripts/python/Perf-Trace-Util/lib/Perf/Trace/SchedGui.py
unknown-mix: ./tools/perf/scripts/python/Perf-Trace-Util/lib/Perf/Trace/Util.py
tabs: ./tools/perf/scripts/python/arm-cs-trace-disasm.py
tabs: ./tools/perf/scripts/python/check-perf-trace.py
tabs: ./tools/perf/scripts/python/compaction-times.py
8spa: ./tools/perf/scripts/python/event_analyzing_sample.py
tabs: ./tools/perf/scripts/python/export-to-postgresql.py
tabs: ./tools/perf/scripts/python/export-to-sqlite.py
tabs: ./tools/perf/scripts/python/exported-sql-viewer.py
tabs: ./tools/perf/scripts/python/failed-syscalls-by-pid.py
4spa: ./tools/perf/scripts/python/flamegraph.py
4spa: ./tools/perf/scripts/python/futex-contention.py
tabs: ./tools/perf/scripts/python/intel-pt-events.py
tabs: ./tools/perf/scripts/python/libxed.py
tabs: ./tools/perf/scripts/python/mem-phys-addr.py
tabs: ./tools/perf/scripts/python/net_dropmonitor.py
tabs: ./tools/perf/scripts/python/netdev-times.py
tabs: ./tools/perf/scripts/python/powerpc-hcalls.py
tabs: ./tools/perf/scripts/python/sched-migration.py
tabs: ./tools/perf/scripts/python/sctop.py
4spa: ./tools/perf/scripts/python/stackcollapse.py
4spa: ./tools/perf/scripts/python/stat-cpi.py
tabs: ./tools/perf/scripts/python/syscall-counts-by-pid.py
tabs: ./tools/perf/scripts/python/syscall-counts.py
4spa: ./tools/perf/scripts/python/task-analyzer.py
4spa: ./tools/perf/tests/attr.py
2spa: ./tools/perf/tests/shell/lib/perf_json_output_lint.py
2spa: ./tools/perf/util/setup.py
tabs: ./tools/power/pm-graph/bootgraph.py
tabs: ./tools/power/pm-graph/sleepgraph.py
4spa: ./tools/power/x86/amd_pstate_tracer/amd_pstate_trace.py
4spa: ./tools/power/x86/intel_pstate_tracer/intel_pstate_tracer.py
tabs: ./tools/rcu/rcu-cbs.py
tabs: ./tools/testing/kunit/kunit.py
tabs: ./tools/testing/kunit/kunit_config.py
tabs: ./tools/testing/kunit/kunit_json.py
tabs: ./tools/testing/kunit/kunit_kernel.py
tabs: ./tools/testing/kunit/kunit_parser.py
tabs: ./tools/testing/kunit/kunit_printer.py
tabs: ./tools/testing/kunit/kunit_tool_test.py
2spa: ./tools/testing/kunit/qemu_config.py
tabs: ./tools/testing/kunit/qemu_configs/alpha.py
tabs: ./tools/testing/kunit/qemu_configs/arm.py
tabs: ./tools/testing/kunit/qemu_configs/arm64.py
tabs: ./tools/testing/kunit/qemu_configs/i386.py
tabs: ./tools/testing/kunit/qemu_configs/powerpc.py
tabs: ./tools/testing/kunit/qemu_configs/riscv.py
tabs: ./tools/testing/kunit/qemu_configs/s390.py
tabs: ./tools/testing/kunit/qemu_configs/sparc.py
tabs: ./tools/testing/kunit/qemu_configs/x86_64.py
tabs: ./tools/testing/kunit/run_checks.py
4spa: ./tools/testing/selftests/bpf/test_bpftool.py
4spa: ./tools/testing/selftests/bpf/test_bpftool_synctypes.py
4spa: ./tools/testing/selftests/bpf/test_offload.py
4spa: ./tools/testing/selftests/drivers/net/mlxsw/sharedbuffer_configuration.py
4spa: ./tools/testing/selftests/drivers/sdsi/sdsi_test.py
2spa: ./tools/testing/selftests/exec/binfmt_script.py
4spa: ./tools/testing/selftests/net/devlink_port_split.py
4spa: ./tools/testing/selftests/net/openvswitch/ovs-dpctl.py
4spa: ./tools/testing/selftests/tc-testing/TdcPlugin.py
4spa: ./tools/testing/selftests/tc-testing/TdcResults.py
4spa: ./tools/testing/selftests/tc-testing/plugin-lib/buildebpfPlugin.py
4spa: ./tools/testing/selftests/tc-testing/plugin-lib/nsPlugin.py
4spa: ./tools/testing/selftests/tc-testing/plugin-lib/rootPlugin.py
4spa: ./tools/testing/selftests/tc-testing/plugin-lib/scapyPlugin.py
4spa: ./tools/testing/selftests/tc-testing/plugin-lib/valgrindPlugin.py
4spa: ./tools/testing/selftests/tc-testing/tdc.py
4spa: ./tools/testing/selftests/tc-testing/tdc_batch.py
2spa: ./tools/testing/selftests/tc-testing/tdc_config.py
unknown-0: ./tools/testing/selftests/tc-testing/tdc_config_local_template.py
4spa: ./tools/testing/selftests/tc-testing/tdc_helper.py
4spa: ./tools/testing/selftests/tc-testing/tdc_multibatch.py
4spa: ./tools/testing/selftests/tpm2/tpm2.py
4spa: ./tools/testing/selftests/tpm2/tpm2_tests.py
4spa: ./tools/verification/dot2/automata.py
4spa: ./tools/verification/dot2/dot2c
4spa: ./tools/verification/dot2/dot2c.py
4spa: ./tools/verification/dot2/dot2k
4spa: ./tools/verification/dot2/dot2k.py

>
> Cheers,
> Miguel
>


-- 
Íñigo Huguet


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v3] Add .editorconfig file for basic formatting
  2023-05-30  7:52       ` Íñigo Huguet
@ 2023-05-30 12:52         ` Joe Perches
  0 siblings, 0 replies; 10+ messages in thread
From: Joe Perches @ 2023-05-30 12:52 UTC (permalink / raw)
  To: Íñigo Huguet, Miguel Ojeda
  Cc: ojeda, masahiroy, jgg, mic, danny, linux-kernel, corbet, linux,
	willy, Andrew Morton

On Tue, 2023-05-30 at 09:52 +0200, Íñigo Huguet wrote:
> Not sure what to do with these 3 (sh, bash, pl): a big majority are
> tab indented, but there are some files with inconsistent indentations
> within the same file (i.e. scripts/get_maintainer.pl). Honestly I
> would be tempted of adding them too.

I'd be annoyed.  I believe that file uses emacs perl
indentation rather consistently.  4 space indentation
with 8 char tab, maximal tab fill.


^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2023-05-30 12:53 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-04-14 10:10 [PATCH v3] Add .editorconfig file for basic formatting Íñigo Huguet
2023-04-14 13:36 ` Miguel Ojeda
2023-05-08  8:58   ` Íñigo Huguet
2023-05-26 21:16     ` Miguel Ojeda
2023-05-30  7:52       ` Íñigo Huguet
2023-05-30 12:52         ` Joe Perches
2023-05-16 13:04 ` Vincent Mailhol
2023-05-18  7:53   ` Íñigo Huguet
2023-05-18  8:59     ` Vincent MAILHOL
2023-05-18  9:16       ` Íñigo Huguet

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).