On 25.10.23 20:50, Jakob Meng wrote: > On 25.10.23 15:48, Mike Pattrick wrote: >> On Wed, Oct 25, 2023 at 9:02 AM Robin Jarry <[email protected]> wrote: >>> Eelco Chaudron, Oct 25, 2023 at 14:56: >>>> On 25 Oct 2023, at 13:52, [email protected] wrote: >>>> >>>>> From: Jakob Meng <[email protected]> >>>>> >>>>> A patch created with 'git format-patch' can contain trailing spaces. >>>>> When editing a patch, e.g. to fix a typo in the title, the trailing >>>>> spaces should not be removed. This becomes tricky when editors like >>>>> Kate identify a space followed by form feed as a trailing space and >>>>> remove both. This will result in a broken patch which cannot be applied >>>>> cleanly. Although this case should likely be considered a editor bug, >>>>> keeping trailing spaces in a patch is the right thing to do and also >>>>> helps mitigating these kinds of editor bugs. >>>>> >>>>> Fixes: 07f6d6a0cb51 ("Add editorconfig file.") >>>> This looks good to me, however as you mention this is more of an >>>> editor configuration issue. It looks like leaving out any default >>>> value will cause the editor to use its configured value. >>>> >>>> Acked-by: Eelco Chaudron <[email protected]> >>> It seems OK as well. But another safer option would have been to move >>> the trim_trailing_whitespace = true option in specific sections. Or >>> completely remove it since it will be caught by checkpatch. >> I think it also makes sense to remove trim_trailing_whitespace from >> the default section, but the current patch makes sense as a standalone >> change. >> >> Acked-by: Mike Pattrick <[email protected]> > Thank you all for your feedback! You are right, I will change my patch ☺️ > > We should completely remove the default section because we cannot set any > reasonable defaults for all possible filetypes. For example, IDEs tend to > write their own files to a subfolder like .vscode or .kdev4. A default > section would apply to files in there, too, which is not safe in general. > > We also should not use insert_final_newline and trim_trailing_whitespace > anywhere in .editorconfig! Editors interpret these lines differently, some > will wipe whitespaces across the whole file, others will only remove them > from lines being edited and others change their behavior between releases. > Limiting those options to a subset like *.c/*.h will not help: As written in > my other response, the definition of whitespace is ambiguous. Unicode > considers form feed to be a whitespace [0], causing some editors to wipe form > feeds when trim_trailing_whitespace is true which is not what we want in OVS. > As Robin mentioned, we already have a test for our definition of whitespaces > and thus we can avoid this whitespace mess by not using it in .editorconfig. > > [0] https://en.wikipedia.org/wiki/Whitespace_character >
Updated patch is out 🥂 https://patchwork.ozlabs.org/project/openvswitch/patch/[email protected]/ _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
