google-java-format icon indicating copy to clipboard operation
google-java-format copied to clipboard

(Prepare for) Handling trailing spaces in multiline strings

Open cgrushko opened this issue 5 years ago • 9 comments

We ran into this issue with ktfmt, and solved it by replacing the last of the trailing spaces with a special tombstone to prevent GJF from removing all trailing whitespace.

https://github.com/facebookincubator/ktfmt/commit/a4fdd39237d2b59ae3c03cee9322ab04ed2b9222

I'm filing this issue so ktfmt can revert to using the supported method once it's available in GJF :) (assuming multiline strings in Java are going to be a thin soon-ish)

cgrushko avatar Mar 26 '20 08:03 cgrushko

I've been looking at supporting Java language features up to 14 recently, including text blocks, but hadn't thought through this yet. It makes sense that we don't want to make non-semantics-preserving changes to multiline strings. OTOH, have you encountered cases where trailing whitespace in multi-line strings is actually desired/useful? It seems like something that might deserve a style rule.

cushon avatar Mar 26 '20 17:03 cushon

Funnily, it tripped our own tests. We have since converted to regular strings to make it more explicit.

Do you suggest the formatter refuses to format such files?

On Thu, Mar 26, 2020 at 7:28 PM Liam Miller-Cushon [email protected] wrote:

I've been looking at supporting Java language features up to 14 recently, including text blocks, but hadn't thought through this yet. It makes sense that we don't want to make non-semantics-preserving changes to multiline strings. OTOH, have you encountered cases where trailing whitespace in multi-line strings is actually desired/useful? It seems like something that might deserve a style rule.

— You are receiving this because you authored the thread.

Reply to this email directly, view it on GitHub https://github.com/google/google-java-format/issues/450#issuecomment-604566364, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAPH6YP5UK464YKSTXHYWG3RJOGB7ANCNFSM4LUB5ARQ .

cgrushko avatar Mar 26 '20 20:03 cgrushko

Do you suggest the formatter refuses to format such files?

No, it generally tries to accept anything that's syntactically valid and produce semantically equivalent output, I was just curious. We should probably try to preserve trailing whitespace and discourage it separately (e.g. with a linter).

cushon avatar Mar 26 '20 20:03 cushon

Cool; I didn't see a test for trailing spaces, though?

On Tue, Mar 31, 2020 at 2:43 PM Christian Stein [email protected] wrote:

Closeable due to 4ddb914 https://github.com/google/google-java-format/commit/4ddb9148c103b9c65768e519884b0c8ad5caaf39 methinks.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/google/google-java-format/issues/450#issuecomment-606576175, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAPH6YM6MBXMKQPG2SHZCVTRKHJO3ANCNFSM4LUB5ARQ .

cgrushko avatar Mar 31 '20 13:03 cgrushko

Cool; I didn't see a test for trailing spaces, though?

Dooh. I replied to the wrong issue and deleted my reply in the meanwhile.

sormuras avatar Mar 31 '20 14:03 sormuras

Why isn't removing trailing white space correct, since it's removed during compilation anyway?

anthonyvdotbe avatar Jun 01 '20 05:06 anthonyvdotbe

I don't trailing white space is removed from raw (multiline) strings during compilation.

On Mon, Jun 1, 2020 at 8:50 AM Anthony Vanelverdinghe < [email protected]> wrote:

Why isn't removing trailing white space correct, since it's removed during compilation anyway?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/google/google-java-format/issues/450#issuecomment-636626759, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAPH6YLLYDCQ6LFN7I4HXSLRUM6QXANCNFSM4LUB5ARQ .

cgrushko avatar Jun 01 '20 06:06 cgrushko

The algorithm described in the JEP has:

  1. Remove all trailing white space [...]

For cases where trailing spaces must be kept, the line can be ended with the new \s escape sequence:

\s can act as fence to prevent the stripping of trailing white space

anthonyvdotbe avatar Jun 01 '20 06:06 anthonyvdotbe

Interesting! makes sense - otherwise removing trailing whitespace from a .java file requires parsing it. So yeah, looks like no need to handle it in GJF. (I don't see something similar for Kotlin, which inspired this issue; I imagine that most editors strip trailing whitespace, so in practice Kotlin programs don't have trailing whitespace)

cgrushko avatar Jun 01 '20 06:06 cgrushko