Re: [PATCH v3] Add .editorconfig file for basic formatting

From: Íñigo Huguet
Date: Thu May 18 2023 - 05:17:44 EST

Next message: Atin Bainada: "[PATCH] media: av7110: Remove unnecessary (void*) conversions"
Previous message: Xia Fukun: "[PATCH v7] kobject: Fix global-out-of-bounds in kobject_action_type()"
In reply to: Vincent MAILHOL: "Re: [PATCH v3] Add .editorconfig file for basic formatting"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

On Thu, May 18, 2023 at 10:59 AM Vincent MAILHOL
<mailhol.vincent@xxxxxxxxxx> wrote:
>
> On Thu. 18 May 2023 at 16:53, Íñigo Huguet <ihuguet@xxxxxxxxxx> wrote:
> > Hi Vincent,
> >
> > On Tue, May 16, 2023 at 3:05 PM Vincent Mailhol
> > <mailhol.vincent@xxxxxxxxxx> 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@xxxxxxxxxx> 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@xxxxxxxxxxx/
> > > > Link: https://lore.kernel.org/lkml/20230404075540.14422-1-ihuguet@xxxxxxxxxx/
> > > > Co-developed-by: Danny Lin <danny@xxxxxxxxxxx>
> > > > Signed-off-by: Danny Lin <danny@xxxxxxxxxxx>
> > > > Signed-off-by: Íñigo Huguet <ihuguet@xxxxxxxxxx>
> > > > ---
> > > > 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@xxxxxxxxxxxxxx/
>
> 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@xxxxxxxxxx/

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@xxxxxxxxxx>
> Tested-by: Vincent Mailhol <mailhol.vincent@xxxxxxxxxx>

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@xxxxxxxxxxxxxx/

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@xxxxxxxxxxx/
> > > > 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

Next message: Atin Bainada: "[PATCH] media: av7110: Remove unnecessary (void*) conversions"
Previous message: Xia Fukun: "[PATCH v7] kobject: Fix global-out-of-bounds in kobject_action_type()"
In reply to: Vincent MAILHOL: "Re: [PATCH v3] Add .editorconfig file for basic formatting"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]