| Message ID | 20241115151030.1198371-2-mailhol.vincent@wanadoo.fr (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | David Ahern |
| Headers | show |
| Series | [iproute2-next] add .editorconfig file for basic formatting | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Not a local patch |
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
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
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
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
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.
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
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
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