[ovs-dev,v3] editorconfig: Remove [*] section and trim_trailing_whitespace.

From: Jakob Meng <code@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 <code@jakobmeng.de>
---
 .editorconfig | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

On 27 Oct 2023, at 14:10, jmeng@redhat.com wrote:

> From: Jakob Meng <code@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 <code@jakobmeng.de>

Thanks Jakob for fixing this! The changes look good to me.


Acked-by: Eelco Chaudron <echaudro@redhat.com>

, Oct 27, 2023 at 14:10:
> From: Jakob Meng <code@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 <code@jakobmeng.de>
> ---
>  .editorconfig | 13 ++++++++-----
>  1 file changed, 8 insertions(+), 5 deletions(-)
>
> diff --git a/.editorconfig b/.editorconfig
> index 685c72750..41ba51bf3 100644
> --- a/.editorconfig
> +++ b/.editorconfig
> @@ -2,15 +2,18 @@
>  
>  root = true
>  
> -[*]
> -end_of_line = lf
> -insert_final_newline = true
> -trim_trailing_whitespace = true
> -charset = utf-8
> +# 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]
> -- 
> 2.39.2

Perfect, thanks Jakob!

Reviewed-by: Robin-Jarry <rjarry@redhat.com>

Hi Jakob,

Just a comment about the use of 'whitespace' in the commit message.
There is an effort to try and use more inclusive language, so it might
be good to use empty-space or blank-space in the commit message.  I know
that it is an editor config property, so not much to do about
trim_trailing_whitespaces in the message.

Additionally, for things like "contain trailing whitespaces" maybe we
can use "contain trailing blank characters?"

Thanks!

jmeng@redhat.com writes:

> From: Jakob Meng <code@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.

Luckily, developers can install a hook that will run checkpatch on a
commit.  Something like the below installed in .git/hooks/pre-commit
should run when trying to commit and catch trailing spaces errors.

------8<------
#!/bin/sh
if git rev-parse --verify HEAD 2>/dev/null
then
    git diff-index -p --cached HEAD
else
    :
fi | utilities/checkpatch.py -s -S
------>8------

> [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 <code@jakobmeng.de>
> ---
>  .editorconfig | 13 ++++++++-----
>  1 file changed, 8 insertions(+), 5 deletions(-)
>
> diff --git a/.editorconfig b/.editorconfig
> index 685c72750..41ba51bf3 100644
> --- a/.editorconfig
> +++ b/.editorconfig
> @@ -2,15 +2,18 @@
>  
>  root = true
>  
> -[*]
> -end_of_line = lf
> -insert_final_newline = true
> -trim_trailing_whitespace = true
> -charset = utf-8
> +# 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]

Hi Aaron,

first and foremost: I am fully committed to make our OVS community more inclusive. Replacing terminology that promotes racial and cultural bias is one of many (!) steps in achieving that. For the objective there is a broad consensus, on the practical implementation not so much, it seems.

IBM Academy of Technology conducted a evaluation of IT terminology and published a guideline on which terms to replace including rationale [0]:

[0] https://github.com/IBM/IBMInclusiveITLanguage

For example, it replaces {black,white}list with {block,allow}list because:

"As a pair, "blacklist" and "whitelist" promote racial bias by implying that black is bad and white is good. When the terms 'white' or 'black' are used in a context where white is represented as good or black is represented as bad, this usage reinforces a model that promotes racial bias." [0]

However, for "white space" there is "No change recommended" because:

"This term is not based on a good/bad binary and so does not fall under our guiding principle for black and white terms (see "blacklist")." [0]


Precision of expression is part of inclusive language. In my commit message I am using the term "white space" because that is exactly the technical term used in Unicode [1], editors and our ecosystem. Using other words such as empty-space or blank-space could be confusing for readers.

[1] https://en.wikipedia.org/wiki/Whitespace_character

Although disturbed at first, I appreciate that you brought this up. We should discuss this with Simon and others to get a common understanding of what inclusive language is in practice. I assume you do not advocate for banning the words white and black completely from the English language, do you?

Best,
Jakob


On 27.10.23 23:53, Aaron Conole wrote:
> Hi Jakob,
>
> Just a comment about the use of 'whitespace' in the commit message.
> There is an effort to try and use more inclusive language, so it might
> be good to use empty-space or blank-space in the commit message.  I know
> that it is an editor config property, so not much to do about
> trim_trailing_whitespaces in the message.
>
> Additionally, for things like "contain trailing whitespaces" maybe we
> can use "contain trailing blank characters?"
>
> Thanks!
>
> jmeng@redhat.com writes:
>
>> From: Jakob Meng <code@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.
> Luckily, developers can install a hook that will run checkpatch on a
> commit.  Something like the below installed in .git/hooks/pre-commit
> should run when trying to commit and catch trailing spaces errors.
>
> ------8<------
> #!/bin/sh
> if git rev-parse --verify HEAD 2>/dev/null
> then
>     git diff-index -p --cached HEAD
> else
>     :
> fi | utilities/checkpatch.py -s -S
> ------>8------
>
>> [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 <code@jakobmeng.de>
>> ---
>>  .editorconfig | 13 ++++++++-----
>>  1 file changed, 8 insertions(+), 5 deletions(-)
>>
>> diff --git a/.editorconfig b/.editorconfig
>> index 685c72750..41ba51bf3 100644
>> --- a/.editorconfig
>> +++ b/.editorconfig
>> @@ -2,15 +2,18 @@
>>  
>>  root = true
>>  
>> -[*]
>> -end_of_line = lf
>> -insert_final_newline = true
>> -trim_trailing_whitespace = true
>> -charset = utf-8
>> +# 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]

On Mon, Oct 30, 2023 at 09:54:58AM +0100, Jakob Meng wrote:
> Hi Aaron,
> 
> first and foremost: I am fully committed to make our OVS community more inclusive. Replacing terminology that promotes racial and cultural bias is one of many (!) steps in achieving that. For the objective there is a broad consensus, on the practical implementation not so much, it seems.
> 
> IBM Academy of Technology conducted a evaluation of IT terminology and published a guideline on which terms to replace including rationale [0]:
> 
> [0] https://github.com/IBM/IBMInclusiveITLanguage
> 
> For example, it replaces {black,white}list with {block,allow}list because:
> 
> "As a pair, "blacklist" and "whitelist" promote racial bias by implying that black is bad and white is good. When the terms 'white' or 'black' are used in a context where white is represented as good or black is represented as bad, this usage reinforces a model that promotes racial bias." [0]
> 
> However, for "white space" there is "No change recommended" because:
> 
> "This term is not based on a good/bad binary and so does not fall under our guiding principle for black and white terms (see "blacklist")." [0]
> 
> 
> Precision of expression is part of inclusive language. In my commit message I am using the term "white space" because that is exactly the technical term used in Unicode [1], editors and our ecosystem. Using other words such as empty-space or blank-space could be confusing for readers.
> 
> [1] https://en.wikipedia.org/wiki/Whitespace_character
> 
> Although disturbed at first, I appreciate that you brought this up. We should discuss this with Simon and others to get a common understanding of what inclusive language is in practice. I assume you do not advocate for banning the words white and black completely from the English language, do you?

FWIIW, based on the above I using the term whitespace seems ok to me.

jmeng@redhat.com writes:

> From: Jakob Meng <code@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 <code@jakobmeng.de>
> ---

Applied, thanks!