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 <c...@jakobmeng.de>
>>
>> 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 <c...@jakobmeng.de>
>> ---
>> .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.
> 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
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
