Message-ID: <31ea1d1b-dbe9-4bc6-8218-64de1884baaf@wanadoo.fr>
Date: Sat, 16 Nov 2024 02:59:01 +0900
From: Vincent Mailhol <mailhol.vincent@...adoo.fr>
To: Stephen Hemminger <stephen@...workplumber.org>
Cc: netdev@...r.kernel.org, David Ahern <dsahern@...il.com>
Subject: Re: [PATCH iproute2-next] add .editorconfig file for basic formatting

On 16/11/2024 at 01:51, Stephen Hemminger wrote:
> On Sat, 16 Nov 2024 00:08:27 +0900
> Vincent Mailhol <mailhol.vincent@...adoo.fr> wrote:
> 
>> EditorConfig is a specification to define the most basic code formatting
>> stuff, and it is 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 configurations to *.{c,h}, *.json or *.yaml.
>>
>> In linux related projects, defining a .editorconfig might help 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.
>>
>> Add the .editorconfig file at the root of the iproute2 project. Only
>> configuration for the file types currently present are specified. The
>> automatic whitespace trimming option caused some issues in the Linux
>> kernel [1] and is thus not activated.
>>
>> See https://editorconfig.org
>>
>> [1] .editorconfig: remove trim_trailing_whitespace option
>> Link: https://git.kernel.org/torvalds/c/7da9dfdd5a3d
>>
>> Signed-off-by: Vincent Mailhol <mailhol.vincent@...adoo.fr>
>> ---
>> For reference, here is the .editorconfig of the kernel:
>>
>>   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/.editorconfig
>> ---
>>  .editorconfig | 24 ++++++++++++++++++++++++
>>  1 file changed, 24 insertions(+)
>>  create mode 100644 .editorconfig
>>
>> diff --git a/.editorconfig b/.editorconfig
>> new file mode 100644
>> index 00000000..4cff39f1
>> --- /dev/null
>> +++ b/.editorconfig
>> @@ -0,0 +1,24 @@
>> +# SPDX-License-Identifier: GPL-2.0
>> +
>> +root = true
> 
> Maybe add something generic across all files. Then you only need to specify overrides

This is risky. Doing this will make the configuration apply to *all*
files with a risk of a "lost bullet". Are sure are we that some editor
will not misinterpret this on some kind of file?

This is why it was decided against it when doing the .editorconfig in
the Linux kernel and that instead, only what we are sure of should be
specified.

Maybe what I can propose as an alternative is to factorize the safe
option and still specify the indentation explicitly depending on the
file type:

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

[{*.{c,h,sh},Makefile}]
indent_style = tab
indent_size = 8

[*.json]
indent_style = space
indent_size = 4

Thoughts?

> [*]
> end_of_line = lf
> insert_final_newline = true
> trim_trailing_whitespace = true

Just let me confirm this one: do you really want the automatic
whitespace removal? On some editor, it will trim not only the modified
lines but also any whitespace in the full file.

This can create "noise" in the patch diff. If you acknowledge this risk,
then I am fine to keep this parameter.

> charset = utf-8
> indent_style = tab
> tab_width = 8
> max_line_length = 100

This max_line_length can also have unexpected consequences. For example,
emacs will apply this parameter to the "fill commands", meaning that,
for example, when editing Markdown or README files, the paragraphs will
be warped at the 100th column. And I do not think that this is the
desired behavior.

If we want to keep the max_line_length, it is better to set it to the
desired default column wrap (e.g. 72 or 80, there is no strong consensus
here as far as I am aware).

>> +
>> +[{*.{c,h,sh},Makefile}]
>> +charset = utf-8
>> +end_of_line = lf
>> +insert_final_newline = true
>> +indent_style = tab
>> +indent_size = 8
>> +
>> +[*.json]
>> +charset = utf-8
>> +end_of_line = lf
>> +insert_final_newline = true
>> +indent_style = space
>> +indent_size = 4
>> +
>> +[*.yaml]
>> +charset = utf-8
>> +end_of_line = lf
>> +insert_final_newline = true
>> +indent_style = space
>> +indent_size = 2
> 

Yours sincerely,
Vincent Mailhol

Powered by blists - more mailing lists