Message-ID: <CACT4oufzafPBOvtYX7W2ossVNeOfBTPwjuce1EG_7YKZN3kpQQ@mail.gmail.com>
Date:   Thu, 18 May 2023 11:16:34 +0200
From:   Íñigo Huguet <ihuguet@...hat.com>
To:     Vincent MAILHOL <mailhol.vincent@...adoo.fr>
Cc:     corbet@....net, danny@...ag0n.dev, jgg@...dia.com, joe@...ches.com,
        linux-kernel@...r.kernel.org, linux@...musvillemoes.dk,
        masahiroy@...nel.org, mic@...ikod.net, ojeda@...nel.org,
        willy@...radead.org
Subject: Re: [PATCH v3] Add .editorconfig file for basic formatting

On Thu, May 18, 2023 at 10:59 AM Vincent MAILHOL
<mailhol.vincent@...adoo.fr> wrote:
>
> On Thu. 18 May 2023 at 16:53, Íñigo Huguet <ihuguet@...hat.com> wrote:
> > Hi Vincent,
> >
> > On Tue, May 16, 2023 at 3:05 PM Vincent Mailhol
> > <mailhol.vincent@...adoo.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@...hat.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@...ag0n.dev>
> > > > Signed-off-by: Danny Lin <danny@...ag0n.dev>
> > > > Signed-off-by: Íñigo Huguet <ihuguet@...hat.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@...adoo.fr>
> Tested-by: Vincent Mailhol <mailhol.vincent@...adoo.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

Powered by blists - more mailing lists