Add clean-up for whitespace to platform repo

[vogella]

@laeubi I would like to add a whitespace removal clean-up to this repo.

I think I saw a discussion between you and @HannesWell as few days ago how we can add a batch-clean-up to a repo. Can you point me to this so that I can see if I can add the whitespace cleanup?

[vogella]

Ah, found it: https://github.com/eclipse-equinox/equinox/pull/936#discussion_r2072648327

[jbampton]

Hell @vogella I would like to work on this issue.

My suggestion is to use pre-commit. It is a popular tool that uses Git hooks. From the pre-commit website:

"Git hook scripts are useful for identifying simple issues before submission to code review. We run our hooks on every commit to automatically point out issues in code such as missing semicolons, trailing whitespace, and debug statements. By pointing these issues out before code review, this allows a code reviewer to focus on the architecture of a change while not wasting time with trivial style nitpicks."

So pre-commit can run on the command line on git commit or with the pre-commit command. Also we can also add pre-commit to the GitHub CI with actions. I have done this on many repos.

https://pre-commit.com/

https://github.com/pre-commit/pre-commit

There are many hooks for pre-commit and some are listed here:

https://pre-commit.com/hooks.html

Also in terms of this issue for whitespace there are some official pre-commit hooks that can help.

All the official hooks are here:

https://github.com/pre-commit/pre-commit-hooks

And for whitespace there is:

• end-of-file-fixer: Makes sure files end in a newline and only a newline
• mixed-line-ending: Replaces or checks mixed line ending.
• trailing-whitespace: trims trailing whitespace

https://github.com/pre-commit/pre-commit-hooks?tab=readme-ov-file#end-of-file-fixer https://github.com/pre-commit/pre-commit-hooks?tab=readme-ov-file#mixed-line-ending https://github.com/pre-commit/pre-commit-hooks?tab=readme-ov-file#trailing-whitespace

So you can run lots of different linters and fixers with pre-commit including:

• actionlint
• codespell
• markdownlint
• markdown-link-check
• oxipng
• shellcheck
• yamllint

and more.

[vogella]

@jbampton I think the solution from @laeubi by using the related JDT clean-up is simpler. Did you check https://github.com/eclipse-equinox/equinox/pull/936#discussion_r2072648327?

[fedejeanne]

So something like https://github.com/eclipse-platform/eclipse.platform.releng.aggregator/pull/3025/ but using cleanup.remove_trailing_whitespaces (or cleanup.remove_trailing_whitespaces_all) instead?

[vogella]

@jbampton do you want to provide a PR for this to the aggregator repo using @fedejeanne suggestion? If yes, please use cleanup.remove_trailing_whitespaces_all to also include empty lines.

To see the clean-ups use right-click on project -> Source -> Clean-up -> Configure

[jbampton]

Hey @vogella thanks for listening to my proposal.

I just started working on another issue.

I will do more PRs for Eclipse in future.

Thanks.

[vogella]

Hey @vogella thanks for listening to my proposal.

Git pre-hooks can be nice but we already have a working solution re-using Eclipse JDT and adding one line to the existing solution seems more efficient than starting an alternative solution.

[HannesWell]

While I'm totally inclined in cleaning up the code, base I think we have to be careful with enabling such clean-ups that potentially create a lot of changes, similar to formatting the code to follow the Eclipse format or adding braces everywhere, as they could shadow previous changes in the git-blame. And unfortunately git-blame is still an important source of information. I think that there is a solution to that problem by using git blame -ignore-rev, but this would then require more coordination.

[vogella]

While I'm totally inclined in cleaning up the code, base I think we have to be careful with enabling such clean-ups that potentially create a lot of changes, similar to formatting the code to follow the Eclipse format or adding braces everywhere, as they could shadow previous changes in the git-blame. And unfortunately git-blame is still an important source of information. I think that there is a solution to that problem by using git blame -ignore-rev, but this would then require more coordination.

Git has the ignore whitespace changes as well as EGit. A long time ago I ran the clean-up whitespaces on all repos I had access to as contributors and committer were complaining about about unnecessary white-space changes and enabled the save-actions for all plug-ins I found relevant. So this setting should ensure this standard is re-applied to the projects.

[merks]

Are you referring the "Show Revision" decorations in an editor or the Blame view on GitHub web page, or just to the button to ignore white space change when comparing state?

[HannesWell]

While I'm totally inclined in cleaning up the code, base I think we have to be careful with enabling such clean-ups that potentially create a lot of changes, similar to formatting the code to follow the Eclipse format or adding braces everywhere, as they could shadow previous changes in the git-blame. And unfortunately git-blame is still an important source of information. I think that there is a solution to that problem by using git blame -ignore-rev, but this would then require more coordination.

Git has the ignore whitespace changes as well as EGit.

I'm not referring to the Git Compare, where one can indeed ignore white-space, I'm referring to Git Blame shown on the left, where one can see the last commit that changed the corresponding line.

A long time ago I ran the clean-up whitespaces on all repos I had access to as contributors and committer were complaining about about unnecessary white-space changes and enabled the save-actions for all plug-ins I found relevant. So this setting should ensure this standard is re-applied to the projects.

If that's the case then all my previous comments can be disregarded, but it should be checked/estimated in advance how much the repos for which this workflow is in enabled are affected.

[vogella]

Example commits in the platform repo are:

5 years ago 15af0e89d20c710f98e4c4305ff011ba9775dcb2 for resources 5 years ago 890f62e121fbde1b3f7c6f17b3d42c4b72c0ce92 for user assistence 5 years ago 8c3dcf24735a8b00029b1be21cc801cf6d9a3d95 for debug

IIRC Ant never got a ws clean-up as the Ant committer were strongly against clean-ups in the past (including the ones @laeubi automated already).

If that's the case then all my previous comments can be disregarded, but it should be checked/estimated in advance how much the repos for which this workflow is in enabled are affected.

I will run some manual whitespace clean-ups and copy you in, so that you can judge the changes.

[fedejeanne]

Git has the ignore whitespace changes as well as EGit.

I'm not referring to the Git Compare, where one can indeed ignore white-space, I'm referring to Git Blame shown on the left, where one can see the last commit that changed the corresponding line.

@HannesWell this view is also affected by the configuration. If you activate this configuration then you don't see the whitespace change in line 39 that Lars made back then.

[HannesWell]

If that's the case then all my previous comments can be disregarded, but it should be checked/estimated in advance how much the repos for which this workflow is in enabled are affected.

I will run some manual whitespace clean-ups and copy you in, so that you can judge the changes.

Thank you. It looks like that indeed mostly blank lines of (empty) javadoc lines are affected. So it seems not much relevant information is lost. In this case I think we can proceed with this as it is.

@HannesWell this view is also affected by the configuration. If you activate this configuration then you don't see the whitespace change in line 39 that Lars made back then.

I wasn't aware that this is already possible, good to know. Thank you for that clarification. In general e.g. reformatting changes are then of course not handled (but that's another story).

[fedejeanne]

I wasn't aware that this is already possible, good to know. Thank you for that clarification. In general e.g. reformatting changes are then of course not handled (but that's another story).

My pleasure.

And thank you for the tip about the git blame --ignore-rev! I didn't know about that but a quick chat with ~ChatGPT~ LeChat (its European cousin 😄) gave me and idea on how to possibly ignore some of the more invasive refactorings by using git blame --ignore-revs-file and some git commit hooks. I'll try to check that out in the coming weeks.

(man lernt nie aus)