On 26.10.23 14:21, Robin Jarry wrote: > Jakob Meng, Oct 26, 2023 at 14:17: >> On 26.10.23 13:52, Robin Jarry wrote: >> > , Oct 26, 2023 at 13:07: >> >> From: Jakob Meng <[email protected]> >> >> >> >> Wildcard sections [*] and [**] are unsafe because properties cannot be >> >> applied safely to any filetype in general. For example, IDEs like >> >> Visual Studio Code and KDevelop store configuration files in subfolders >> >> like .vscode or .kdev4. Properties from wildcard sections also apply to >> >> those files which is not safe in general. >> >> Another example are patches created with 'git format-patch' which can >> >> contain trailing whitespaces. When editing a patch, e.g. to fix a typo >> >> in the title, trailing whitespaces should not be removed. >> >> >> >> Property trim_trailing_whitespace should not be defined at all because >> >> it is interpreted differently by editors. Some wipe whitespaces from >> >> the whole file, others remove them from edited lines only and a few >> >> change their behavior between releases [0]. Limiting the property to a >> >> subset of files like *.c/*.h will not mitigate the issue: >> >> >> >> Multiple definitions of a whitespace exist. Unicode considers a form >> >> feed (0x0C) to be a whitespace [1]. QChar::isSpace() [2] from Qt >> >> follows this definition, causing the Kate editor identify a form feed >> >> as a trailing whitespace and removing it from sources [3]. This breaks >> >> patches when editors remove form feeds and thus causing broken patches >> >> which cannot be applied cleanly. >> >> >> >> Removing trim_trailing_whitespace will be a minor inconvienence, in >> >> particular because utilities/checkpatch.py and thus 0-day Robot will >> >> prevent trailing whitespaces for our definition of a whitespace. >> >> >> >> [0] >> >> https://github.com/KDE/ktexteditor/commit/94b328fc64e543d91930845d2a96ce08d3043295 >> >> [1] https://en.wikipedia.org/wiki/Whitespace_character >> >> [2] >> >> https://github.com/qt/qtbase/blob/5628600a07295db6ed6683e97fafb0c45ddea505/src/corelib/text/qchar.h#L554 >> >> [3] >> >> https://github.com/KDE/ktexteditor/blob/10210ec1dd06afa1e7b19a4fff722a8a23719161/src/document/katedocument.cpp#L5643 >> >> >> >> Fixes: 07f6d6a0cb51 ("Add editorconfig file.") >> >> >> >> Signed-off-by: Jakob Meng <[email protected]> >> >> --- >> >> .editorconfig | 34 +++++++++++++++++++++++++++++----- >> >> 1 file changed, 29 insertions(+), 5 deletions(-) >> >> >> >> diff --git a/.editorconfig b/.editorconfig >> >> index 685c72750..aebcf3a72 100644 >> >> --- a/.editorconfig >> >> +++ b/.editorconfig >> >> @@ -2,47 +2,71 @@ >> >> >> >> root = true >> >> >> >> -[*] >> >> -end_of_line = lf >> >> -insert_final_newline = true >> >> -trim_trailing_whitespace = true >> >> -charset = utf-8 >> > >> > Hi Jakob, >> > >> > I think you could keep these two options: >> > >> > end_of_line = lf >> > charset = utf-8 >> > >> >> You cannot decide this in general for all possible filetypes across all >> possible dev platforms. Again, those properties in [*] would also apply to >> non-ovs-owned files inside the source tree, e.g. created by IDEs. Unsafe. >> Please don't. > > Ok fair enough. > >> > And probably adding insert_final_newline = true is not necessary. > >> > checkpatch should complain if the final newline is missing. >> >> I left it in .editorconfig because it is not causing trouble. But I can >> remove it, if you want. > > I don't think it is important you can leave it or remove it. > > However I just realized that you could simply copy the settings in the > [*.{c,h}] section as other sections will inherit from it unless they override > something.
Oh, nice! IIUC this is actually covered by the EditorConfig spec [0]: "All found EditorConfig files are searched for sections with section names matching the given filename." and "Files are read top to bottom and the most recent rules found take precedence. If multiple EditorConfig files have matching sections, the rules from the closer EditorConfig file are read last, so pairs in closer files take precedence." and "For any pair, a value of unset removes the effect of that pair, even if it has been set before. For example, add indent_size = unset to undefine the indent_size pair (and use editor defaults)." The latter would not make sense if you could not inherit properties from other sections. [0] https://spec.editorconfig.org/ > >> > Your patch should only remove insert_final_newline and > >> > trim_trailing_whitespace from the default section. >> >> >> > >> >> +# No wildcard sections [*] and [**] because properties cannot be >> >> +# applied safely to any filetype in general. >> >> + >> >> +# Property trim_trailing_whitespace should not be defined at all >> >> +# because it is interpreted differently by editors. >> >> >> >> [*.{c,h}] >> >> +charset = utf-8 >> >> +end_of_line = lf >> >> indent_style = space >> >> indent_size = 4 >> >> +insert_final_newline = true >> >> max_line_length = 79 >> >> >> >> [include/linux/**.h] >> >> +charset = utf-8 >> >> +end_of_line = lf >> >> indent_style = tab >> >> indent_size = tab >> >> +insert_final_newline = true >> >> tab_width = 8 >> >> >> >> [include/sparse/rte_*.h] >> >> +charset = utf-8 >> >> +end_of_line = lf >> >> indent_style = tab >> >> +insert_final_newline = true >> >> tab_width = 8 >> >> >> >> [include/windows/getopt.h] >> >> +charset = utf-8 >> >> +end_of_line = lf >> >> indent_style = tab >> >> indent_size = tab >> >> +insert_final_newline = true >> >> tab_width = 8 >> >> >> >> [include/windows/netinet/{icmp6,ip6}.h] >> >> +charset = utf-8 >> >> +end_of_line = lf >> >> indent_style = tab >> >> indent_size = tab >> >> +insert_final_newline = true >> >> tab_width = 8 >> >> >> >> [lib/getopt_long.c] >> >> +charset = utf-8 >> >> +end_of_line = lf >> >> indent_style = tab >> >> indent_size = tab >> >> +insert_final_newline = true >> >> tab_width = 8 >> >> >> >> [lib/sflow*.{c,h}] >> >> +charset = utf-8 >> >> +end_of_line = lf >> >> indent_style = tab >> >> indent_size = tab >> >> +insert_final_newline = true >> >> tab_width = 8 >> >> >> >> [lib/strsep.c] >> >> +charset = utf-8 >> >> +end_of_line = lf >> >> indent_style = tab >> >> indent_size = tab >> >> +insert_final_newline = true >> >> tab_width = 8 >> >> -- >> >> 2.39.2 >> > >> > >> > > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
