On 26 Oct 2023, at 14:34, Jakob Meng wrote:
> 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/ So I guess we can expect a new rev. Will mark this as changes requested for now. >> >>>> 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
