[PATCH iproute2-next] add .editorconfig file for basic formatting

[PATCH iproute2-next] add .editorconfig file for basic formatting

[Vincent Mailhol]

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@wanadoo.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
+
+[{*.{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
-- 
2.45.2

Re: [PATCH iproute2-next] add .editorconfig file for basic formatting

[Stephen Hemminger]

On Sat, 16 Nov 2024 00:08:27 +0900
Vincent Mailhol <mailhol.vincent@wanadoo.fr> wrote:

> +
> +[*.yaml]
> +charset = utf-8
> +end_of_line = lf
> +insert_final_newline = true
> +indent_style = space
> +indent_size = 2
> -- 

Ok, but there are no .yaml files in iproute2-next

Re: [PATCH iproute2-next] add .editorconfig file for basic formatting

[Vincent Mailhol]

On 16/11/2024 at 01:49, Stephen Hemminger wrote:
> On Sat, 16 Nov 2024 00:08:27 +0900
> Vincent Mailhol <mailhol.vincent@wanadoo.fr> wrote:
> 
>> +
>> +[*.yaml]
>> +charset = utf-8
>> +end_of_line = lf
>> +insert_final_newline = true
>> +indent_style = space
>> +indent_size = 2
>> -- 
> 
> Ok, but there are no .yaml files in iproute2-next

My bad, I had a .yaml file in my local directory generated by some
static analysis tool.

Will be removed in v2.


Yours sincerely,
Vincent Mailhol

Re: [PATCH iproute2-next] add .editorconfig file for basic formatting

[Stephen Hemminger]

On Sat, 16 Nov 2024 00:08:27 +0900
Vincent Mailhol <mailhol.vincent@wanadoo.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@wanadoo.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

[*]
end_of_line = lf
insert_final_newline = true
trim_trailing_whitespace = true
charset = utf-8
indent_style = tab
tab_width = 8
max_line_length = 100

> +
> +[{*.{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

Re: [PATCH iproute2-next] add .editorconfig file for basic formatting

[Vincent Mailhol]

On 16/11/2024 at 01:51, Stephen Hemminger wrote:
> On Sat, 16 Nov 2024 00:08:27 +0900
> Vincent Mailhol <mailhol.vincent@wanadoo.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@wanadoo.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

Re: [PATCH iproute2-next] add .editorconfig file for basic formatting

[Stephen Hemminger]

On Sat, 16 Nov 2024 02:59:01 +0900
Vincent Mailhol <mailhol.vincent@wanadoo.fr> wrote:

> > [*]
> > 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.


Yes, emacs and some other editors have bad habit of leaving trailing
whitespace. And sometimes new files get added without new line at end.

Line length should be 100 like kernel.

Re: [PATCH iproute2-next] add .editorconfig file for basic formatting

[Vincent Mailhol]

On Sat. 16 Nov. 2024 at 05:32, Stephen Hemminger
<stephen@networkplumber.org> wrote:
> On Sat, 16 Nov 2024 02:59:01 +0900
> Vincent Mailhol <mailhol.vincent@wanadoo.fr> wrote:
>
> > > [*]
> > > 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.
>
>
> Yes, emacs and some other editors have bad habit of leaving trailing
> whitespace. And sometimes new files get added without new line at end.

Ack. trim_trailing_whitespace = true will be added back in v2.

> Line length should be 100 like kernel.

It is a bit more nuanced.

  1. On the code, the official limit remains 80:

       https://www.kernel.org/doc/html/latest/process/coding-style.html#breaking-long-lines-and-strings

     Since below commit on checkpatch.pl:

       https://git.kernel.org/torvalds/c/bdc48fa11e46

     warnings are indeed only produced for exceeding the 100th column,
     but as far as I can see, the 80 column limit is still broadly
     followed on kdoc comment blocks and on all the documentation.

  2 .Commit are wrapped at 75 columns:

       https://www.kernel.org/doc/html/latest/process/submitting-patches.html#the-canonical-patch-format

    When I applied the max_line_length = 100 parameter, emacs started
    wrapping the body of my commits to the 100th column. I will add:

      [COMMIT_EDITMSG]
      max_line_length = 75

    to the v2 to prevent this.

In v2, I will apply your suggestion of a broad max_line_length = 100
configuration, with a max_line_length = 75 exception for the commit
messages. I am happy to take any of your suggestions as long as you
agree on the risk and potential side effects!

Yours sincerely,
Vincent Mailhol