Skip to content
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

Detect and preserve line separators instead of using system default #340

Closed
wants to merge 6 commits into from

Conversation

ParkerM
Copy link
Contributor

@ParkerM ParkerM commented Aug 25, 2022

Fixes #280

The current behavior of Formatter#format falls back to the system default line separator when one isn't explicitly specified, causing a bunch of false positives when building on Windows machines with LF configured via .gitconfig or .gitattributes.
This modifies the behavior to instead detect a common line separator per source file, and only fall back to system default when one cannot be determined for some reason.

(See linked issue for additional context)

@philwebb
Copy link
Contributor

@philwebb philwebb added this to the 0.0.36 milestone Feb 15, 2023
@philwebb
Copy link
Contributor

@ParkerM I'm in the process of merging this one, I've got a local branch that replaces the regex. I'm not sure it's worth actually reusing the eclipse code directly.

@ParkerM
Copy link
Contributor Author

ParkerM commented Feb 16, 2023

10-4, here's a backup in case anything was clobbered by my recent push: https://github.com/ParkerM/spring-javaformat/tree/backup/detect-eol

Thanks for looking into it!

philwebb pushed a commit that referenced this pull request Feb 16, 2023
Update `Formatter` so that line endings are now detected based on
the contents of the file. This mirrors the behavior of the Eclipse
plugin when used in the IDE, specifically the `nextDelimiterInfo`
method in `org.eclipse.jface.text.DefaultLineTracker`.

See gh-340
philwebb added a commit that referenced this pull request Feb 16, 2023
@philwebb philwebb closed this in 097bb33 Feb 16, 2023
@ParkerM ParkerM deleted the detect-eol branch February 16, 2023 05:14
@philwebb
Copy link
Contributor

Thanks very much for the PR @ParkerM. I've now merged this with a few changes to the tests which hopefully mean we don't need .gitattributes. I was a little worried that someone would edit the file and the line-endings would change.

@ParkerM
Copy link
Contributor Author

ParkerM commented Feb 16, 2023

Right on, good call on the test changes (and double good call on swapping out the clumsy regex impl). Just ran a local build on my Windows 10 box and everything's green! 🥳

@philwebb philwebb self-assigned this Feb 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

System-default line separators are implicitly enforced despite lax configurations
2 participants