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

astgen: forbid trailing whitespace in multiline strings #20575

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

matklad
Copy link
Contributor

@matklad matklad commented Jul 10, 2024

Trailing whitespace in multiline strings is annoying for all the usual
reasons trailing whitespace is annoying (unrelated line changes).

However, in this case it even can leak into whatever the string literal
is used for. For example, in the previous commit there is trailing
whitespace leaking into the help messages.

So it makes sense to forbid this outright.

zig fmt intentionally doesn't try to clean up here --- if there's a
trailing whitespace in a multiline literal, it is ambiguous whether
the user added that by accident, or whether they intended to have a
space there.

If you need to have trailing whitespace, you can contatenate \\ and
" strings with ++:

const S =
    \\hello
++ "  \n" ++
    \\world
    \\
;

Closes: #19299

@matklad
Copy link
Contributor Author

matklad commented Jul 10, 2024

Note: that proposal is neither accepted not rejected. This PR shouldn't be considered as an argument in favor of accepting it. Rather, if we do accept it, than that's how an implementation could look like.

That being said, I am strongly in favor of accepting this, as I've hit the problem of trailing whitespace leakage a couple of times.

@matklad matklad force-pushed the matklad/no-trailing-whitespace branch 2 times, most recently from 61af27a to 797b1db Compare July 10, 2024 12:33
@matklad matklad requested a review from squeek502 as a code owner July 10, 2024 12:33
matklad added 2 commits July 10, 2024 16:13
We are going to forbid this shortly
Trailing whitespace in multiline strings is annoying for all the usual
reasons trailing whitespace is annoying (unrelated line changes).

However, in this case it even can leak into whatever the string literal
is used for. For example, in the previous commit there is trailing
whitespace leaking into the help messages.

So it makes sense to forbid this outright.

`zig fmt` intentionally doesn't try to clean up here --- if there's a
trailing whitespace in a multiline literal, it is _ambiguous_ whether
the user added that by accident, or whether they _intended_ to have a
space there.

If you need to have trailing whitespace, you can contatenate `\\` and
`"` strings with `++`:

    const S =
        \\hello
    ++ "  \n" ++
        \\world
        \\
    ;

Closes: ziglang#19299
@matklad matklad force-pushed the matklad/no-trailing-whitespace branch from 797b1db to 4476ff4 Compare July 10, 2024 15:14
@andrewrk
Copy link
Member

OK, I'll make an effort to reach a decision on that proposal since you already have a working PR 👍

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

Successfully merging this pull request may close these issues.

Make trailing whitespace at the end of multiline strings an error
2 participants