Re: [webkit-dev] Removal of trailing whitespace

[Anne van Kesteren via webkit-dev]

On Wed, Apr 12, 2023 at 7:20 PM Yusuke Suzuki via webkit-dev
 wrote:
> I agree that we should not do it because it pollutes change history of files, 
> git-blame results, and review-diff in PR.
> But at the same time, I think there is no reason to add a new trailing 
> whitespace via a new commit.
> It is nice if we can enforce this rule only for newly added code (via 
> style-checker) not to add new trailing spaces.

As it appears this particular change has agreement from several
people, I created a PR that hopefully implements it:
https://github.com/WebKit/WebKit/pull/13441. This does not touch
.editorconfig as I'm pretty sure that always ends up applying to the
entire modified file.

I would like to thank everyone for their input and at the same time
apologize for the whitespace removal in my earlier set of patches.
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev

Re: [webkit-dev] Removal of trailing whitespace

[Anthony Ricaud via webkit-dev]

(not a stakeholder either)

On top of ignoring whitespace, Git and Github support ignoring commits when 
performing `git blame`.

With that tool, one commit could remove trailing whitespace over the all 
project and add some editorconfig or CI checks to prevent them from coming 
back. Then, a second commit would add the first commit to the ignore-revs-file.

https://git-scm.com/docs/git-blame#Documentation/git-blame.txt---ignore-revs-fileltfilegt

https://docs.github.com/en/repositories/working-with-files/using-files/viewing-a-file#ignore-commits-in-the-blame-view

> On 13 Apr 2023, at 08:43, Mathias Bynens via webkit-dev 
>  wrote:
> 
> (not a stakeholder; just curious)
> 
> Does it help that Git supports the `-w` / `--ignore-all-space` flag to ignore 
> whitespace changes? It works with `git diff`, `git blame`, etc.
> 
> GitHub also supports it — compare e.g.
> 
> https://github.com/WebKit/WebKit/commit/2ca55a3e19df60127dec09f3af22e4d3ab2943ec
> vs.
> https://github.com/WebKit/WebKit/commit/2ca55a3e19df60127dec09f3af22e4d3ab2943ec?w=1
> 
> On Wed, Apr 12, 2023 at 9:35 PM Chris Dumez via webkit-dev 
> mailto:webkit-dev@lists.webkit.org>> wrote:
>> 
>> 
>>> On Apr 12, 2023, at 10:23 AM, Ryosuke Niwa via webkit-dev 
>>> mailto:webkit-dev@lists.webkit.org>> wrote:
>>> 
>>> Yeah, enforcing that new or otherwise modified lines don’t have trailing 
>>> whitespaces would be good.
>> 
>> Yes, I wouldn’t mind that either.
>> 
>> However, https://commits.webkit.org/262879@main has just landed and if you 
>> look at the changes to Document.cpp, it is mostly spacing changes :(
>> It makes it harder to review or to identify meaningful changes in a patch 
>> after landing. It also pollutes git blame for no great reason.
>> 
>>> 
>>> - R. Niwa
>>> 
 On Apr 12, 2023, at 10:20 AM, Yusuke Suzuki >>> > wrote:
 
 I agree that we should not do it because it pollutes change history of 
 files, git-blame results, and review-diff in PR.
 But at the same time, I think there is no reason to add a new trailing 
 whitespace via a new commit.
 It is nice if we can enforce this rule only for newly added code (via 
 style-checker) not to add new trailing spaces.
 
 -Yusuke
 
> On Apr 12, 2023, at 10:08 AM, Ryosuke Niwa via webkit-dev 
> mailto:webkit-dev@lists.webkit.org>> wrote:
> 
> WebKi proejctt’s long term policy has been to not do this:
> https://lists.webkit.org/pipermail/webkit-dev/2009-August/009665.html
> 
> I don’t think we should change that.
> 
> - R. Niwa
> 
>> On Apr 12, 2023, at 9:17 AM, Chris Dumez via webkit-dev 
>> mailto:webkit-dev@lists.webkit.org>> wrote:
>> 
>> I am against this because it adds a lot of noise to patches I am trying 
>> to review.
>> I have seen PRs where white space changes account for more than half the 
>> patch I am trying to review.
>> 
>> Dropping trailing spaces on the lines you’re modifying is OK but in the 
>> whole file is too noisy IMO.
>> 
>> Chris.
>> 
>>> On Apr 12, 2023, at 1:22 AM, Anne van Kesteren via webkit-dev 
>>> mailto:webkit-dev@lists.webkit.org>> 
>>> wrote:
>>> 
>>> To reduce the overhead of switching between projects with different
>>> whitespace requirements, I would like to suggest we start being
>>> lenient when trailing whitespace is removed. In particular when a file
>>> is being changed to fix a bug.
>>> 
>>> I could see going even further and enforcing this via the style
>>> checker, if there is appetite for that.
>>> 
>>> Thanks for considering!
>>> ___
>>> webkit-dev mailing list
>>> webkit-dev@lists.webkit.org 
>>> https://lists.webkit.org/mailman/listinfo/webkit-dev
>> 
>> ___
>> webkit-dev mailing list
>> webkit-dev@lists.webkit.org 
>> https://lists.webkit.org/mailman/listinfo/webkit-dev
> 
> ___
> webkit-dev mailing list
> webkit-dev@lists.webkit.org 
> https://lists.webkit.org/mailman/listinfo/webkit-dev
 
>>> 
>>> ___
>>> webkit-dev mailing list
>>> webkit-dev@lists.webkit.org 
>>> https://lists.webkit.org/mailman/listinfo/webkit-dev
>> 
>> ___
>> webkit-dev mailing list
>> webkit-dev@lists.webkit.org 
>> https://lists.webkit.org/mailman/listinfo/webkit-dev
> ___
> webkit-dev mailing list
> webkit-dev@lists.webkit.org
> https://lists.webkit.org/mailman/listinfo/webkit-dev

___
webkit-dev mailing list

Re: [webkit-dev] Removal of trailing whitespace

[Vitor Ribeiro Roriz via webkit-dev]

Besides enforcing no whitespaces in new lines. Are PRs exclusively removing 
whitespaces from a file (or a batch of files) welcome? 

> On 13. Apr 2023, at 12:20, Adrian Perez de Castro via webkit-dev 
>  wrote:
> 
> Hi,
> 
> On Wed, 12 Apr 2023 10:23:06 -0700 Ryosuke Niwa via webkit-dev 
>  wrote:
> 
>> Yeah, enforcing that new or otherwise modified lines don’t have trailing
>> whitespaces would be good.
> 
> I very much agree with this. Let's leave existing code as-is, having new and
> modified lines without trailing whitespace. It's nice to avoid trailing
> whitespace, but better not at the cost of littering diffs with unrelated
> changes that only touch whitespace.
> 
> 
> Cheers,
> —Adrián
> ___
> webkit-dev mailing list
> webkit-dev@lists.webkit.org
> https://lists.webkit.org/mailman/listinfo/webkit-dev

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev

Re: [webkit-dev] Removal of trailing whitespace

[Adrian Perez de Castro via webkit-dev]

Hi,

On Wed, 12 Apr 2023 10:23:06 -0700 Ryosuke Niwa via webkit-dev 
 wrote:

> Yeah, enforcing that new or otherwise modified lines don’t have trailing
> whitespaces would be good.

I very much agree with this. Let's leave existing code as-is, having new and
modified lines without trailing whitespace. It's nice to avoid trailing
whitespace, but better not at the cost of littering diffs with unrelated
changes that only touch whitespace.


Cheers,
—Adrián


signature.asc
Description: PGP signature
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev

Re: [webkit-dev] Removal of trailing whitespace

[Adrian Perez de Castro via webkit-dev]

Hi,

On Wed, 12 Apr 2023 20:27:25 -0500 Michael Catanzaro via webkit-dev 
 wrote:
> On Thu, Apr 13 2023 at 08:15:00 AM +0900, Tetsuharu Ohzeki via 
> webkit-dev  wrote:
>
> > To digress a little, why does webkit now use a style checker based on 
> > python script instead of clang-format?
> > In today, I feel it's more reasonable to use such a formetter.
> 
> We've tried clang-format in several GNOME projects with not great 
> results. I'd recommend uncrustify instead.

That may have been the case because the conventions used for code style
in GLib/GObject-adjacent projects is quite peculiar, and uncrustify allows
for a bit more tweaking than clang-format (and even so I have seen some
projects use additional scripts for the API headers, which have their own
set of indentation challenges!). OTOH, clang-format supports the WebKit
C++ coding conventions out of the box (“clang-format --style=WebKit”).

> Still, I'm not sure it's a good idea for WebKit. I'm sure we could make 
> either tool work, but we'd have to be very lax with any configuration 
> we use, or it could get pretty annoying. And the existing style checker 
> works decently enough.

Personally, I do not care that much which particular style checker is
used, as long as there is a sanctioned style and some way of checking it.
The current style checker is fine, and supports more languages than C/C++
so in that regard it's better than clang-format.

Cheers,
—Adrián


signature.asc
Description: PGP signature
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev

Re: [webkit-dev] Removal of trailing whitespace

[Mathias Bynens via webkit-dev]

(not a stakeholder; just curious)

Does it help that Git supports the `-w` / `--ignore-all-space` flag to
ignore whitespace changes? It works with `git diff`, `git blame`, etc.

GitHub also supports it — compare e.g.

https://github.com/WebKit/WebKit/commit/2ca55a3e19df60127dec09f3af22e4d3ab2943ec
vs.
https://github.com/WebKit/WebKit/commit/2ca55a3e19df60127dec09f3af22e4d3ab2943ec?w=1

On Wed, Apr 12, 2023 at 9:35 PM Chris Dumez via webkit-dev <
webkit-dev@lists.webkit.org> wrote:

>
>
> On Apr 12, 2023, at 10:23 AM, Ryosuke Niwa via webkit-dev <
> webkit-dev@lists.webkit.org> wrote:
>
> Yeah, enforcing that new or otherwise modified lines don’t have trailing
> whitespaces would be good.
>
>
> Yes, I wouldn’t mind that either.
>
> However, https://commits.webkit.org/262879@main has just landed and if
> you look at the changes to Document.cpp, it is mostly spacing changes :(
> It makes it harder to review or to identify meaningful changes in a patch
> after landing. It also pollutes git blame for no great reason.
>
>
> - R. Niwa
>
> On Apr 12, 2023, at 10:20 AM, Yusuke Suzuki  wrote:
>
> I agree that we should not do it because it pollutes change history of
> files, git-blame results, and review-diff in PR.
> But at the same time, I think there is no reason to add a new trailing
> whitespace via a new commit.
> It is nice if we can enforce this rule only for newly added code (via
> style-checker) not to add new trailing spaces.
>
> -Yusuke
>
> On Apr 12, 2023, at 10:08 AM, Ryosuke Niwa via webkit-dev <
> webkit-dev@lists.webkit.org> wrote:
>
> WebKi proejctt’s long term policy has been to not do this:
> https://lists.webkit.org/pipermail/webkit-dev/2009-August/009665.html
>
> I don’t think we should change that.
>
> - R. Niwa
>
> On Apr 12, 2023, at 9:17 AM, Chris Dumez via webkit-dev <
> webkit-dev@lists.webkit.org> wrote:
>
> I am against this because it adds a lot of noise to patches I am trying to
> review.
> I have seen PRs where white space changes account for more than half the
> patch I am trying to review.
>
> Dropping trailing spaces on the lines you’re modifying is OK but in the
> whole file is too noisy IMO.
>
> Chris.
>
> On Apr 12, 2023, at 1:22 AM, Anne van Kesteren via webkit-dev <
> webkit-dev@lists.webkit.org> wrote:
>
> To reduce the overhead of switching between projects with different
> whitespace requirements, I would like to suggest we start being
> lenient when trailing whitespace is removed. In particular when a file
> is being changed to fix a bug.
>
> I could see going even further and enforcing this via the style
> checker, if there is appetite for that.
>
> Thanks for considering!
> ___
> webkit-dev mailing list
> webkit-dev@lists.webkit.org
> https://lists.webkit.org/mailman/listinfo/webkit-dev
>
>
> ___
> webkit-dev mailing list
> webkit-dev@lists.webkit.org
> https://lists.webkit.org/mailman/listinfo/webkit-dev
>
>
> ___
> webkit-dev mailing list
> webkit-dev@lists.webkit.org
> https://lists.webkit.org/mailman/listinfo/webkit-dev
>
>
>
> ___
> webkit-dev mailing list
> webkit-dev@lists.webkit.org
> https://lists.webkit.org/mailman/listinfo/webkit-dev
>
>
> ___
> webkit-dev mailing list
> webkit-dev@lists.webkit.org
> https://lists.webkit.org/mailman/listinfo/webkit-dev
>
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev

Re: [webkit-dev] Removal of trailing whitespace

[Fujii Hironori via webkit-dev]

I checked the clang-format result of WTF.
$ find Source/WTF -name '*.h' -o -name '*.cpp' -exec clang-format -i '{}'
';'
Although it doesn't comply with the current WebKit style, it looks good
enough to me.
By adopting clang-format for the project, we can forget most parts of
WebKit style guidelines.
It can reduce the memory footprint for WebKit contributors.

However, inline asm are formatted badly, we should disable it for them.
// clang-format off
(..)
// clang-format on

On Thu, Apr 13, 2023 at 10:28 AM Michael Catanzaro via webkit-dev <
webkit-dev@lists.webkit.org> wrote:

> On Thu, Apr 13 2023 at 08:15:00 AM +0900, Tetsuharu Ohzeki via
> webkit-dev  wrote:
> > To digress a little, why does webkit now use a style checker based on
> > python script instead of clang-format?
> > In today, I feel it's more reasonable to use such a formetter.
>
> We've tried clang-format in several GNOME projects with not great
> results. I'd recommend uncrustify instead.
>
> Still, I'm not sure it's a good idea for WebKit. I'm sure we could make
> either tool work, but we'd have to be very lax with any configuration
> we use, or it could get pretty annoying. And the existing style checker
> works decently enough.
>
>
> ___
> webkit-dev mailing list
> webkit-dev@lists.webkit.org
> https://lists.webkit.org/mailman/listinfo/webkit-dev
>
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev

Re: [webkit-dev] Removal of trailing whitespace

[Michael Catanzaro via webkit-dev]

On Thu, Apr 13 2023 at 08:15:00 AM +0900, Tetsuharu Ohzeki via 
webkit-dev  wrote:
To digress a little, why does webkit now use a style checker based on 
python script instead of clang-format?

In today, I feel it's more reasonable to use such a formetter.


We've tried clang-format in several GNOME projects with not great 
results. I'd recommend uncrustify instead.


Still, I'm not sure it's a good idea for WebKit. I'm sure we could make 
either tool work, but we'd have to be very lax with any configuration 
we use, or it could get pretty annoying. And the existing style checker 
works decently enough.



___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev

Re: [webkit-dev] Removal of trailing whitespace

[Tetsuharu Ohzeki via webkit-dev]

To digress a little, why does webkit now use a style checker based on
python script instead of clang-format?
In today, I feel it's more reasonable to use such a formetter.

(This is just a off topic & just my interesting about history,
I don’t have a intention to propose in this discussion to switch the tool
to check and sort the coding style)


2023年4月13日(木) 4:40 Ryosuke Niwa via webkit-dev :

>
> On Apr 12, 2023, at 12:34 PM, Chris Dumez  wrote:
>
> On Apr 12, 2023, at 10:23 AM, Ryosuke Niwa via webkit-dev <
> webkit-dev@lists.webkit.org> wrote:
>
> Yeah, enforcing that new or otherwise modified lines don’t have trailing
> whitespaces would be good.
>
>
> Yes, I wouldn’t mind that either.
>
> However, https://commits.webkit.org/262879@main has just landed and if
> you look at the changes to Document.cpp, it is mostly spacing changes :(
> It makes it harder to review or to identify meaningful changes in a patch
> after landing. It also pollutes git blame for no great reason.
>
>
> Yeah, it’s not great that PR got landed. In the future, it would be good
> to hold off landing these code changes until the discussion settles.
>
> - R. Niwa
>
> On Apr 12, 2023, at 10:20 AM, Yusuke Suzuki  wrote:
>
> I agree that we should not do it because it pollutes change history of
> files, git-blame results, and review-diff in PR.
> But at the same time, I think there is no reason to add a new trailing
> whitespace via a new commit.
> It is nice if we can enforce this rule only for newly added code (via
> style-checker) not to add new trailing spaces.
>
> -Yusuke
>
> On Apr 12, 2023, at 10:08 AM, Ryosuke Niwa via webkit-dev <
> webkit-dev@lists.webkit.org> wrote:
>
> WebKi proejctt’s long term policy has been to not do this:
> https://lists.webkit.org/pipermail/webkit-dev/2009-August/009665.html
>
> I don’t think we should change that.
>
> - R. Niwa
>
> On Apr 12, 2023, at 9:17 AM, Chris Dumez via webkit-dev <
> webkit-dev@lists.webkit.org> wrote:
>
> I am against this because it adds a lot of noise to patches I am trying to
> review.
> I have seen PRs where white space changes account for more than half the
> patch I am trying to review.
>
> Dropping trailing spaces on the lines you’re modifying is OK but in the
> whole file is too noisy IMO.
>
> Chris.
>
> On Apr 12, 2023, at 1:22 AM, Anne van Kesteren via webkit-dev <
> webkit-dev@lists.webkit.org> wrote:
>
> To reduce the overhead of switching between projects with different
> whitespace requirements, I would like to suggest we start being
> lenient when trailing whitespace is removed. In particular when a file
> is being changed to fix a bug.
>
> I could see going even further and enforcing this via the style
> checker, if there is appetite for that.
>
> Thanks for considering!
> ___
> webkit-dev mailing list
> webkit-dev@lists.webkit.org
> https://lists.webkit.org/mailman/listinfo/webkit-dev
>
>
> ___
> webkit-dev mailing list
> webkit-dev@lists.webkit.org
> https://lists.webkit.org/mailman/listinfo/webkit-dev
>
>
> ___
> webkit-dev mailing list
> webkit-dev@lists.webkit.org
> https://lists.webkit.org/mailman/listinfo/webkit-dev
>
>
>
> ___
> webkit-dev mailing list
> webkit-dev@lists.webkit.org
> https://lists.webkit.org/mailman/listinfo/webkit-dev
>
>
>
> ___
> webkit-dev mailing list
> webkit-dev@lists.webkit.org
> https://lists.webkit.org/mailman/listinfo/webkit-dev
>
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev

Re: [webkit-dev] Removal of trailing whitespace

[Ryosuke Niwa via webkit-dev]

> On Apr 12, 2023, at 12:34 PM, Chris Dumez  wrote:
> 
>> On Apr 12, 2023, at 10:23 AM, Ryosuke Niwa via webkit-dev 
>>  wrote:
>> 
>> Yeah, enforcing that new or otherwise modified lines don’t have trailing 
>> whitespaces would be good.
> 
> Yes, I wouldn’t mind that either.
> 
> However, https://commits.webkit.org/262879@main has just landed and if you 
> look at the changes to Document.cpp, it is mostly spacing changes :(
> It makes it harder to review or to identify meaningful changes in a patch 
> after landing. It also pollutes git blame for no great reason.

Yeah, it’s not great that PR got landed. In the future, it would be good to 
hold off landing these code changes until the discussion settles.

- R. Niwa

>>> On Apr 12, 2023, at 10:20 AM, Yusuke Suzuki  wrote:
>>> 
>>> I agree that we should not do it because it pollutes change history of 
>>> files, git-blame results, and review-diff in PR.
>>> But at the same time, I think there is no reason to add a new trailing 
>>> whitespace via a new commit.
>>> It is nice if we can enforce this rule only for newly added code (via 
>>> style-checker) not to add new trailing spaces.
>>> 
>>> -Yusuke
>>> 
 On Apr 12, 2023, at 10:08 AM, Ryosuke Niwa via webkit-dev 
  wrote:
 
 WebKi proejctt’s long term policy has been to not do this:
 https://lists.webkit.org/pipermail/webkit-dev/2009-August/009665.html
 
 I don’t think we should change that.
 
 - R. Niwa
 
> On Apr 12, 2023, at 9:17 AM, Chris Dumez via webkit-dev 
>  wrote:
> 
> I am against this because it adds a lot of noise to patches I am trying 
> to review.
> I have seen PRs where white space changes account for more than half the 
> patch I am trying to review.
> 
> Dropping trailing spaces on the lines you’re modifying is OK but in the 
> whole file is too noisy IMO.
> 
> Chris.
> 
>> On Apr 12, 2023, at 1:22 AM, Anne van Kesteren via webkit-dev 
>>  wrote:
>> 
>> To reduce the overhead of switching between projects with different
>> whitespace requirements, I would like to suggest we start being
>> lenient when trailing whitespace is removed. In particular when a file
>> is being changed to fix a bug.
>> 
>> I could see going even further and enforcing this via the style
>> checker, if there is appetite for that.
>> 
>> Thanks for considering!
>> ___
>> webkit-dev mailing list
>> webkit-dev@lists.webkit.org
>> https://lists.webkit.org/mailman/listinfo/webkit-dev
> 
> ___
> webkit-dev mailing list
> webkit-dev@lists.webkit.org
> https://lists.webkit.org/mailman/listinfo/webkit-dev
 
 ___
 webkit-dev mailing list
 webkit-dev@lists.webkit.org
 https://lists.webkit.org/mailman/listinfo/webkit-dev
>>> 
>> 
>> ___
>> webkit-dev mailing list
>> webkit-dev@lists.webkit.org
>> https://lists.webkit.org/mailman/listinfo/webkit-dev
> 

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev

Re: [webkit-dev] Removal of trailing whitespace

[Chris Dumez via webkit-dev]

> On Apr 12, 2023, at 10:23 AM, Ryosuke Niwa via webkit-dev 
>  wrote:
> 
> Yeah, enforcing that new or otherwise modified lines don’t have trailing 
> whitespaces would be good.

Yes, I wouldn’t mind that either.

However, https://commits.webkit.org/262879@main has just landed and if you look 
at the changes to Document.cpp, it is mostly spacing changes :(
It makes it harder to review or to identify meaningful changes in a patch after 
landing. It also pollutes git blame for no great reason.

> 
> - R. Niwa
> 
>> On Apr 12, 2023, at 10:20 AM, Yusuke Suzuki  wrote:
>> 
>> I agree that we should not do it because it pollutes change history of 
>> files, git-blame results, and review-diff in PR.
>> But at the same time, I think there is no reason to add a new trailing 
>> whitespace via a new commit.
>> It is nice if we can enforce this rule only for newly added code (via 
>> style-checker) not to add new trailing spaces.
>> 
>> -Yusuke
>> 
>>> On Apr 12, 2023, at 10:08 AM, Ryosuke Niwa via webkit-dev 
>>>  wrote:
>>> 
>>> WebKi proejctt’s long term policy has been to not do this:
>>> https://lists.webkit.org/pipermail/webkit-dev/2009-August/009665.html
>>> 
>>> I don’t think we should change that.
>>> 
>>> - R. Niwa
>>> 
 On Apr 12, 2023, at 9:17 AM, Chris Dumez via webkit-dev 
  wrote:
 
 I am against this because it adds a lot of noise to patches I am trying to 
 review.
 I have seen PRs where white space changes account for more than half the 
 patch I am trying to review.
 
 Dropping trailing spaces on the lines you’re modifying is OK but in the 
 whole file is too noisy IMO.
 
 Chris.
 
> On Apr 12, 2023, at 1:22 AM, Anne van Kesteren via webkit-dev 
>  wrote:
> 
> To reduce the overhead of switching between projects with different
> whitespace requirements, I would like to suggest we start being
> lenient when trailing whitespace is removed. In particular when a file
> is being changed to fix a bug.
> 
> I could see going even further and enforcing this via the style
> checker, if there is appetite for that.
> 
> Thanks for considering!
> ___
> webkit-dev mailing list
> webkit-dev@lists.webkit.org
> https://lists.webkit.org/mailman/listinfo/webkit-dev
 
 ___
 webkit-dev mailing list
 webkit-dev@lists.webkit.org
 https://lists.webkit.org/mailman/listinfo/webkit-dev
>>> 
>>> ___
>>> webkit-dev mailing list
>>> webkit-dev@lists.webkit.org
>>> https://lists.webkit.org/mailman/listinfo/webkit-dev
>> 
> 
> ___
> webkit-dev mailing list
> webkit-dev@lists.webkit.org
> https://lists.webkit.org/mailman/listinfo/webkit-dev

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev

Re: [webkit-dev] Removal of trailing whitespace

[Ryosuke Niwa via webkit-dev]

Yeah, enforcing that new or otherwise modified lines don’t have trailing 
whitespaces would be good.

- R. Niwa

> On Apr 12, 2023, at 10:20 AM, Yusuke Suzuki  wrote:
> 
> I agree that we should not do it because it pollutes change history of files, 
> git-blame results, and review-diff in PR.
> But at the same time, I think there is no reason to add a new trailing 
> whitespace via a new commit.
> It is nice if we can enforce this rule only for newly added code (via 
> style-checker) not to add new trailing spaces.
> 
> -Yusuke
> 
>> On Apr 12, 2023, at 10:08 AM, Ryosuke Niwa via webkit-dev 
>>  wrote:
>> 
>> WebKi proejctt’s long term policy has been to not do this:
>> https://lists.webkit.org/pipermail/webkit-dev/2009-August/009665.html
>> 
>> I don’t think we should change that.
>> 
>> - R. Niwa
>> 
>>> On Apr 12, 2023, at 9:17 AM, Chris Dumez via webkit-dev 
>>>  wrote:
>>> 
>>> I am against this because it adds a lot of noise to patches I am trying to 
>>> review.
>>> I have seen PRs where white space changes account for more than half the 
>>> patch I am trying to review.
>>> 
>>> Dropping trailing spaces on the lines you’re modifying is OK but in the 
>>> whole file is too noisy IMO.
>>> 
>>> Chris.
>>> 
 On Apr 12, 2023, at 1:22 AM, Anne van Kesteren via webkit-dev 
  wrote:
 
 To reduce the overhead of switching between projects with different
 whitespace requirements, I would like to suggest we start being
 lenient when trailing whitespace is removed. In particular when a file
 is being changed to fix a bug.
 
 I could see going even further and enforcing this via the style
 checker, if there is appetite for that.
 
 Thanks for considering!
 ___
 webkit-dev mailing list
 webkit-dev@lists.webkit.org
 https://lists.webkit.org/mailman/listinfo/webkit-dev
>>> 
>>> ___
>>> webkit-dev mailing list
>>> webkit-dev@lists.webkit.org
>>> https://lists.webkit.org/mailman/listinfo/webkit-dev
>> 
>> ___
>> webkit-dev mailing list
>> webkit-dev@lists.webkit.org
>> https://lists.webkit.org/mailman/listinfo/webkit-dev
> 

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev

Re: [webkit-dev] Removal of trailing whitespace

[Yusuke Suzuki via webkit-dev]

I agree that we should not do it because it pollutes change history of files, 
git-blame results, and review-diff in PR.
But at the same time, I think there is no reason to add a new trailing 
whitespace via a new commit.
It is nice if we can enforce this rule only for newly added code (via 
style-checker) not to add new trailing spaces.

-Yusuke

> On Apr 12, 2023, at 10:08 AM, Ryosuke Niwa via webkit-dev 
>  wrote:
> 
> WebKi proejctt’s long term policy has been to not do this:
> https://lists.webkit.org/pipermail/webkit-dev/2009-August/009665.html
> 
> I don’t think we should change that.
> 
> - R. Niwa
> 
>> On Apr 12, 2023, at 9:17 AM, Chris Dumez via webkit-dev 
>>  wrote:
>> 
>> I am against this because it adds a lot of noise to patches I am trying to 
>> review.
>> I have seen PRs where white space changes account for more than half the 
>> patch I am trying to review.
>> 
>> Dropping trailing spaces on the lines you’re modifying is OK but in the 
>> whole file is too noisy IMO.
>> 
>> Chris.
>> 
>>> On Apr 12, 2023, at 1:22 AM, Anne van Kesteren via webkit-dev 
>>>  wrote:
>>> 
>>> To reduce the overhead of switching between projects with different
>>> whitespace requirements, I would like to suggest we start being
>>> lenient when trailing whitespace is removed. In particular when a file
>>> is being changed to fix a bug.
>>> 
>>> I could see going even further and enforcing this via the style
>>> checker, if there is appetite for that.
>>> 
>>> Thanks for considering!
>>> ___
>>> webkit-dev mailing list
>>> webkit-dev@lists.webkit.org
>>> https://lists.webkit.org/mailman/listinfo/webkit-dev
>> 
>> ___
>> webkit-dev mailing list
>> webkit-dev@lists.webkit.org
>> https://lists.webkit.org/mailman/listinfo/webkit-dev
> 
> ___
> webkit-dev mailing list
> webkit-dev@lists.webkit.org
> https://lists.webkit.org/mailman/listinfo/webkit-dev

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev

Re: [webkit-dev] Removal of trailing whitespace

[Ryosuke Niwa via webkit-dev]

WebKi proejctt’s long term policy has been to not do this:
https://lists.webkit.org/pipermail/webkit-dev/2009-August/009665.html

I don’t think we should change that.

- R. Niwa

> On Apr 12, 2023, at 9:17 AM, Chris Dumez via webkit-dev 
>  wrote:
> 
> I am against this because it adds a lot of noise to patches I am trying to 
> review.
> I have seen PRs where white space changes account for more than half the 
> patch I am trying to review.
> 
> Dropping trailing spaces on the lines you’re modifying is OK but in the whole 
> file is too noisy IMO.
> 
> Chris.
> 
>> On Apr 12, 2023, at 1:22 AM, Anne van Kesteren via webkit-dev 
>>  wrote:
>> 
>> To reduce the overhead of switching between projects with different
>> whitespace requirements, I would like to suggest we start being
>> lenient when trailing whitespace is removed. In particular when a file
>> is being changed to fix a bug.
>> 
>> I could see going even further and enforcing this via the style
>> checker, if there is appetite for that.
>> 
>> Thanks for considering!
>> ___
>> webkit-dev mailing list
>> webkit-dev@lists.webkit.org
>> https://lists.webkit.org/mailman/listinfo/webkit-dev
> 
> ___
> webkit-dev mailing list
> webkit-dev@lists.webkit.org
> https://lists.webkit.org/mailman/listinfo/webkit-dev

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev

Re: [webkit-dev] Removal of trailing whitespace

[Chris Dumez via webkit-dev]

I am against this because it adds a lot of noise to patches I am trying to 
review.
I have seen PRs where white space changes account for more than half the patch 
I am trying to review.

Dropping trailing spaces on the lines you’re modifying is OK but in the whole 
file is too noisy IMO.

Chris.

> On Apr 12, 2023, at 1:22 AM, Anne van Kesteren via webkit-dev 
>  wrote:
> 
> To reduce the overhead of switching between projects with different
> whitespace requirements, I would like to suggest we start being
> lenient when trailing whitespace is removed. In particular when a file
> is being changed to fix a bug.
> 
> I could see going even further and enforcing this via the style
> checker, if there is appetite for that.
> 
> Thanks for considering!
> ___
> webkit-dev mailing list
> webkit-dev@lists.webkit.org
> https://lists.webkit.org/mailman/listinfo/webkit-dev

___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev

Re: [webkit-dev] Removal of trailing whitespace

[Matthieu Dubet via webkit-dev]

We have an .editorconfig in the repository already, we could/should take
advantage of it.
There has been a commit in 2020 to not enforce trailing whitespace removal,
but we can change it to enforce non-change of trailing whitespace.

https://github.com/WebKit/WebKit/commit/85195ac3385176f0e2b664ad3198c506ef6f

https://github.com/editorconfig/editorconfig/wiki/EditorConfig-Properties#trim_trailing_whitespace

Best,
Matthieu

On Wed, Apr 12, 2023 at 10:22 AM Anne van Kesteren via webkit-dev <
webkit-dev@lists.webkit.org> wrote:

> To reduce the overhead of switching between projects with different
> whitespace requirements, I would like to suggest we start being
> lenient when trailing whitespace is removed. In particular when a file
> is being changed to fix a bug.
>
> I could see going even further and enforcing this via the style
> checker, if there is appetite for that.
>
> Thanks for considering!
> ___
> webkit-dev mailing list
> webkit-dev@lists.webkit.org
> https://lists.webkit.org/mailman/listinfo/webkit-dev
>
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev

Re: [webkit-dev] Removal of trailing whitespace

[Tetsuharu OHZEKI via webkit-dev]

How about adding the trim_trailing_whitespace option to the root's
.editorconfig as the first step?

- https://github.com/WebKit/WebKit/blob/main/.editorconfig
- https://editorconfig.org/

This does not enforce anything for all of us, so we still require some
style checker mechanism,
but I think it's a nice first step for this topic.

--
Tetsuharu OHZEKI
tetsuharu.ohz...@gmail.com

On Wed, Apr 12, 2023 at 5:22 PM Anne van Kesteren via webkit-dev
 wrote:
>
> To reduce the overhead of switching between projects with different
> whitespace requirements, I would like to suggest we start being
> lenient when trailing whitespace is removed. In particular when a file
> is being changed to fix a bug.
>
> I could see going even further and enforcing this via the style
> checker, if there is appetite for that.
>
> Thanks for considering!
> ___
> webkit-dev mailing list
> webkit-dev@lists.webkit.org
> https://lists.webkit.org/mailman/listinfo/webkit-dev
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-dev