8311043: Remove trailing blank lines in source files

[lkorinth]

Remove trailing "blank" lines in source files.

I like to use global-whitespace-cleanup-mode, but I can not use it if the files are "dirty" to begin with. This fix will make more files "clean". I also considered adding a check for this in jcheck for Skara, however it seems jcheck code handling hunks does not track end-of-files. Thus I will only clean the files.

The fix removes trailing lines matching ^[[:space:]]*$ in

• *.java
• *.cpp
• *.hpp
• *.c
• *.h

I have applied the following bash script to each file:

file="$1"

while [[ $(tail -n 1 "$file") =~ ^[[:space:]]*$ ]]; do
    truncate -s -1 "$file"
done

git diff --ignore-space-change --ignore-blank-lines master displays no changes
git diff --ignore-blank-lines master displays one change

---

Progress

• Change must be properly reviewed (1 review required, with at least 1 Reviewer)
• Change must not contain extraneous whitespace
• Commit message must refer to an issue

Issue

• JDK-8311043: Remove trailing blank lines in source files (Enhancement - P4)

Reviewers

• Erik Joelsson (@erikj79 - Reviewer)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/14698/head:pull/14698
$ git checkout pull/14698

Update a local copy of the PR:
$ git checkout pull/14698
$ git pull https://git.openjdk.org/jdk.git pull/14698/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 14698

View PR using the GUI difftool:
$ git pr show -t 14698

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/14698.diff

Webrev

Link to Webrev Comment

[bridgekeeper]

👋 Welcome back lkorinth! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

[openjdk]

@lkorinth The following labels will be automatically applied to this pull request:

• build
• client
• compiler
• core-libs
• graal
• hotspot
• i18n
• javadoc
• jmx
• kulla
• net
• nio
• security
• serviceability
• shenandoah

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing lists. If you would like to change these labels, use the /label pull request command.

[mlbridge]

Webrevs

• 00: Full (a02f91ba)

[openjdk]

@lkorinth This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8311043: Remove trailing blank lines in source files

Reviewed-by: erikj

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 16 new commits pushed to the master branch:

• b2eae16: 8295191: IR framework timeout options expect ms instead of s
• af319d9: 8311064: Windows builds fail without precompiled headers after JDK-8310728
• cbf418a: 8311020: Typo cleanup in Classfile API
• f4b900b: 8310902: (fc) FileChannel.transferXXX async close and interrupt issues
• cf8d706: 8308463: Refactor regenerated class handling in lambdaFormInvokers.cpp
• 6f58ab2: 8301569: jmod list option and jimage list --help not interpreted correctly on turkish locale
• 8f5a384: 8311032: Empty value for java.protocol.handler.pkgs system property can lead to unnecessary classloading attempts of protocol handlers
• ded1370: 8309811: BytecodePrinter cannot handle unlinked classes
• 02b17d7: 8310264: In PhaseChaitin::Split defs and phis are leaked
• a63afa4: 8294427: Check boxes and radio buttons have rendering issues on Windows in High DPI env
• ... and 6 more: https://git.openjdk.org/jdk/compare/9f98136c3a00ca24d59ffefd58308603b58110c7...master

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

[dholmes-ora]

This seems to run contrary to the requirement that files end in a newline, which git will complain about if the newline is missing.

It also seems far too large and disruptive.

[erikj79]

This seems to run contrary to the requirement that files end in a newline, which git will complain about if the newline is missing.

The patch is leaving exactly one newline at the end of the file.

[lkorinth]

This seems to run contrary to the requirement that files end in a newline, which git will complain about if the newline is missing.

It also seems far too large and disruptive.

Do you still think it is too disruptive after Erik's explanation? I could split it in more reviews, but I thought that maybe it would just make the review harder. I was hoping the two git diff commands I gave in my first comment in combination with the clean script would make the change seem reviewable.

[coleenp]

Why do we care about this?

[dholmes-ora]

Neither the PR diffs nor the webrev make it easy to see exactly what is being changed here. It appeared to me that the last empty line of each file was being deleted, leaving no newline at the end.

But to me this is too disruptive with no tangible benefit. And you need buy-in from all the different areas affected by this.

[lkorinth]

Why do we care about this?

I care because of global-whitespace-cleanup-mode (in emacs). It helps me remove trailing whitespaces and blanklines when saving but it will not fix a file that was "dirty" when it was opened. Trailing blank lines triggers it not to clean whitespaces for me.

And it does not look good.

[coleenp]

You could fix your emacs functions.

[lkorinth]

Neither the PR diffs nor the webrev make it easy to see exactly what is being changed here. It appeared to me that the last empty line of each file was being deleted, leaving no newline at the end.

My changes look like this in the diff output

 }
-

Removal of the last newline would look like this:

-}
+}
\ No newline at end of file

(both with git diff and git diff --unified)

I have not tested if this is also true for the generated webrevs, but I think that is precisely how they are created.

[lkorinth]

You could fix your emacs functions.

It is a very nice feature of global-whitespace-cleanup-mode

[coleenp]

Per had an emacs feature to remove whitespaces at the end of the line, and gave me the vim version of that. That's a nice feature. I object to this change.

[lkorinth]

This was not liked, I will close it.

I will possibly do it under another PR for the GC code.

Thanks for reviewing.

[dholmes-ora]

My changes look like this in the diff output

 }
-

Thanks for showing this and other output. To me this looked like the final newline had been removed. I would have expected to see something that more obviously showed more than one blank line before hand and exactly one after.

[dcubed-ojdk]

Ending the file with a blank line? I would not have expected that at all.
I expect a single EOL at the end of the file; from a visual POV, the last
line of non-blank text ends with an EOL. No more, no less.