Skip to content

Google Java Format doesn't reflow long strings #924

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

Closed
tbroyer opened this issue Sep 1, 2021 · 4 comments · Fixed by #929
Closed

Google Java Format doesn't reflow long strings #924

tbroyer opened this issue Sep 1, 2021 · 4 comments · Fixed by #929

Comments

@tbroyer
Copy link
Contributor

tbroyer commented Sep 1, 2021

Using either Maven (2.12.3) or Gradle (5.14.3), with various versions of Google Java Format (including the latest, 1.11.0), Spotless doesn't reflow long strings.

This is similar to google/google-java-format#566 and google/google-java-format#642 for the GJF IntelliJ and Eclipse plugins.

Spotless would have to explicitly call StringWrapper.wrap(). This has been added in GJF 1.8.

Maybe there should be a configuration flag to disable it, similar to GJF's command-line --skip-reflowing-long-strings.

@nedtwigg
Copy link
Member

nedtwigg commented Sep 1, 2021

Because this would be a behavior change, it would need to be optin. E.g. googleJavaFormat().reflowLongStrings(true). Happy to merge a PR for it.

@tbroyer
Copy link
Contributor Author

tbroyer commented Sep 2, 2021

Fwiw, when unused imports removal and import ordering were added (in 3.0.0, GJF being available since 2.2.0), it wasn't added as an opt-in. Though I could understand making it opt-in until the next major release.

@nedtwigg
Copy link
Member

nedtwigg commented Sep 2, 2021

it wasn't added as an opt-in

Just wanna make sure I understand correctly. If I make a table of GJF command-line jar versions and whether they reflow strings, it would look like this:

  • before 1.8: does not reflow long String literals
  • 1.8 and later: does reflow long String literals

And if I made a table of GJF in Spotless, the table would look like this:

  • all combinations of Spotless and GJF: does not reflow String literals

Is that correct? If so, then I think from the perspective of Spotless users, we definitely want the reflowLongStrings(false) to be the default behavior, now and in the future.

I can see a case for switching the default to match GJF in a future breaking version, so that people switching from CLI to Spotless have better defaults. But Spotless has a lot of users, and the promise we have made to them is "if you adopt us, you can think about formatting less, and when other formatters misbehave or change their defaults, we will insulate you from that." Helping people adopt is good, but most of all I want to help the people who have adopted to forget that we exist, and one way we do that is we minimize changes of defaults.

@tbroyer
Copy link
Contributor Author

tbroyer commented Sep 2, 2021

Is that correct?

Yes.

tbroyer added a commit to tbroyer/spotless that referenced this issue Sep 4, 2021
Added configuration option to switch the option of the
googleJavaFormat step.
Defaults to not reflowing for backwards compatibility.

Fixes diffplug#924
tbroyer added a commit to tbroyer/spotless that referenced this issue Sep 4, 2021
Add configuration option to switch the option of the
googleJavaFormat step.
Default to not reflowing for backwards compatibility.

Fixes diffplug#924
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants