Received-SPF: pass (google.com: domain of linux-kernel+bounces-198325-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Date: Sun, 2 Jun 2024 17:30:51 +0200
From: Mateusz Guzik <mjguzik@gmail.com>
To: Masahiro Yamada <masahiroy@kernel.org>
Cc: =?utf-8?B?w43DsWlnbw==?= Huguet <ihuguet@redhat.com>, 
	Vincent MAILHOL <mailhol.vincent@wanadoo.fr>, Jonathan Corbet <corbet@lwn.net>, ojeda@kernel.org, 
	danny@kdrag0n.dev, jgg@nvidia.com, mic@digikod.net, linux-kernel@vger.kernel.org, 
	joe@perches.com, linux@rasmusvillemoes.dk, willy@infradead.org
Subject: Re: [PATCH v4] Add .editorconfig file for basic formatting
Message-ID: <thbrfziusf37nj5mwsj2a6zmjtenj5b5vhzwu2z5zkhr7ajsg6@whvx46y6mxbz>
References: <20230601075333.14021-1-ihuguet@redhat.com>
 <CAMZ6RqLoRVHD_M8Jh2ELurhL8E=HWt2DZZFGQvmfFyxKjtNKhg@mail.gmail.com>
 <874jiikr6e.fsf@meer.lwn.net>
 <CAMZ6RqLJmTjM0dYvixMEAo+uW+zfhdL1n4rnajsHCZcq971oRA@mail.gmail.com>
 <CACT4oudYAK07+PJzJMhTazKe3LP-F4tpQf8CF0vs1pJLEE_4aA@mail.gmail.com>
 <CAK7LNATqNNVX8ECNoO82kY503ArfRPa9GdbYd9tOok_6tGOsew@mail.gmail.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <CAK7LNATqNNVX8ECNoO82kY503ArfRPa9GdbYd9tOok_6tGOsew@mail.gmail.com>

On Tue, Dec 12, 2023 at 02:50:59AM +0900, Masahiro Yamada wrote:
> On Mon, Oct 23, 2023 at 3:26 PM Íñigo Huguet <ihuguet@redhat.com> wrote:
> >
> > On Mon, Oct 23, 2023 at 8:19 AM Vincent MAILHOL
> > <mailhol.vincent@wanadoo.fr> wrote:
> > >
> > > On Mon. 23 Oct. 2023 at 11:28, Jonathan Corbet <corbet@lwn.net> wrote:
> > > > Vincent MAILHOL <mailhol.vincent@wanadoo.fr> writes:
> > > >
> > > > > On Thu. 1 June 2023 at 16:53, Íñ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
> > > > >>
> > > > >> Co-developed-by: Danny Lin <danny@kdrag0n.dev>
> > > > >> Signed-off-by: Danny Lin <danny@kdrag0n.dev>
> > > > >> Signed-off-by: Íñigo Huguet <ihuguet@redhat.com>
> > > > >> ---
> > > > >
> > > > > Is there any news for this patch? I would really love this to become mainstream.
> > > >
> > > > I have concerns about this patch that I have expressed in the past.
> > > >
> > > > I'm not going to apply it... since it's a global change that affects all
> > > > kernel developers, I don't think I *should* apply it.  I would recommend
> > > > sending it directly to Linus; if you can get an ack from him, I'll apply
> > > > it then.
> > >
> > > Hi Jonathan,
> > >
> > > Thanks for the comment, message taken.
> > >
> > > Hi Íñigo,
> > >
> > > The last version of the patch being from you, would you like to bring
> > > the topic to Linus yourself or shall I do it instead?
> >
> > I'm not doing kernel development lately, so please go ahead pushing
> > this if you want.
> >
> > Anyway, note that, as discussed in the thread, it is incorrect to say
> > that it will affect all kernel developers: most IDEs and editors only
> > have "opt-in" support for editorconfig, and the few that are not
> > "opt-in", are either "opt-out" or has a workaround (Kate).
> 
> 
> 
> 
> Applied to linux-kbuild. Thanks.
> 
> We invested a significant amount of time in research and discussion,
> and I do not want to lose our efforts.
> 
> If it turns out to be problematic, it is easy to revert it.
> (although I do not think so, given the wide adoption in many projects.)
> 

So after recently switching to neovim I found that .editorconfig results
in messing up my diffs.

I'm screwing around in the vfs layer which has numerous pre-existing
whitespace issues. neovim proceeds to *silently* whackall of the stray
spaces and whatnot, which you only find out about after you git diff/git
show

You can try yourself by editing fs/dcache.c or fs/bad_inode.c.

It stems from this line:
trim_trailing_whitespace = true

The problem is dodged by setting it to false or adding this to
~/.config/nvim/init.lua:
vim.g.editorconfig = false

Is there a non-giant-hammer method to disable the thing? Maybe some lua
magic to special-case that this is the kernel tree?

Also note editing the shipped config shows as a diff to master, so doing
it is not that great either.

If someone could block new whitespace issues from appearing *and* sorted
out all existing ones, I would have no comments. Definitely not going to
try even myself though.

A naive/shite grep claims 840 .c files with issues:
find . -name '*.c' -print0 | xargs -0 grep -l ' $' | wc -l

As is I think the config mostly gets in the way and most people have a
setup which does not use it (for example my vim does not). Alternatively
maye it is neovim which is overzalous here and other editors don't treat
the entire file. If there is a way to convince the thing to only tend to
whitespace issues in edited lines that would be great.

I think the best way forward for the time being is to move .editorconfig
somewhere (scripts/ even though it is not one?) and leave it there as a
template for interested parties. Then people who insist on using the
config can cp and modify (or not) as needed.

I don't have a strong opinion as long as it buggers off the whitespace I
did not even touch or a solution is given which whacks this aspect, but
only for the Linux repo.