-
Notifications
You must be signed in to change notification settings - Fork 861
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
update formatter to handle record closing braces correctly #8076
Conversation
to show the effect of propery spaceswithinMethodDeclaration on record. See issue apache#7043. moved Sample spaces from bundle to text block. To show effect of apache#7043. added test RecordFormattingTest to show problem \apache#7043 There are two tests in the test class, one that passws with the spaceWithinMethodDeclParens set to false, and one that fails when this is set to true.
solves missing space before closing RPAREN in record.
Indetantion of the closing brace of a record was off and dependent of preference spaceWithinMethodDeclParens. fixed in Java Source Base/org.netbeans.modules.java.source.save.Reformatter. Tests added in file org.netbeans.modules.java.source.save.RecordFormattingTest
Added solution to the problem. This solves #7043 |
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.
hi @homberghp, first of all: thanks for working on this and contributing the fix for the bug you found.
If I see this correctly, the changeset (outside of the tests) are about 2 lines, rest are whitespace changes. We usually try to avoid making unrelated changes when fixing bugs.
Please update the changes to only those lines which matter for the fix - feel free to squash and force push into this PR.
// copied from Bundle.properties for easier maintanance | ||
static String sampleSpaces =""" |
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.
we shouldn't inline message bundle values, I suppose this was only done for debugging purposes? Changes to this file should be dropped IMO.
System.err.println("golden length = " + golden.trim().length()); | ||
System.err.println("content length = " + content.trim().length()); |
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.
no println in tests please. We have hours of tests and have to keep the output manageable
@mbien: thank you for the time spent reviewing my pull request. I will close this pull request and create a new one, based on your suggestions. |
#7043 is still open, due to lacking tests.
Here is the test.