-
Notifications
You must be signed in to change notification settings - Fork 70
Java files always end with a new line #1433
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Generate changelog in
|
✅ Successfully generated changelog entry!What happened?Your changelog entries have been stored in the database as part of our migration to ChangelogV3. Need to regenerate?Simply interact with the changelog bot comment again to regenerate these entries. |
| // Ensure the file ends with a newline | ||
| builder.forcedBreak(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we write a test for this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also perhaps the multiple lines at the end of the file case
|
Adding |
| case "--character-range": | ||
| case "-character-range": | ||
| parseRangeSet(optionsBuilder.characterRangesBuilder(), getValue(flag, it, value)); | ||
| parseCharacterRanges(optionsBuilder.characterRangesBuilder(), getValue(flag, it, value)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug from the beginning of pjf. parseRangeSet expects a 1-indexed range, but we have a 0-indexed range (see RnageUtils class).
This would mean that Intellij only (./gradlew spotless would work correclty as it doesn't use the same getReplacements workflow) would not reformat the full file eg. after formatting the "format.input" file, we'd get a truncated version of the formatted output:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice find!
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
intentionally removing the last line to make sure we are getting the error seen in: https://github.com/palantir/palantir-java-format/pull/1433/files#r2429564482
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is great, should remove this toil for devs. I checked as well and intellij will format after making a new file, so this is immediately correct.
|
Released 2.79.0 |
Before this PR
Checkstyle requires Java files to end with a new line see here.
After this PR
We are now enforcing this through the formatter directly.
Fixed a bug where Bootstrapping/NativeImageFormatter were incorrectly reading the input range as a 1-index vs 0-index.
==COMMIT_MSG==
Java files always end with a new line
==COMMIT_MSG==
Possible downsides?