Handling trailing whitespace in Heredoc #8692
Conversation
marcandre
force-pushed
the
whitespace
branch
from
a1953b9
to
fd9922c
Compare
|
Looks like I need to review my PR, some of these trailing whitespaces were actually a mistake. It doesn't invalidate the rest of the discussion (quite the contrary) |
marcandre
force-pushed
the
whitespace
branch
from
fd9922c
to
322932e
Compare
marcandre
force-pushed
the
whitespace
branch
3 times, most recently
from
266f397
to
9a46876
Compare
|
So I found 10 useless trailing whitespaces in our own code... I tweaked the auto-correction to not do anything for static heredocs |
marcandre
force-pushed
the
whitespace
branch
from
9a46876
to
4eb904d
Compare
marcandre
force-pushed
the
whitespace
branch
4 times, most recently
from
61e69c2
to
91ddd7a
Compare
I agree with the general premise, but I'm wondering if that won't be a bit confusing. It would certainly confuse me if I didn't read your explanations in the PR's description. :-)
Fine by me. |
|
I also think that if we go down this road it'd be nice to expand the cop description with some of the rationale outlined in the PR's description. |
|
Another possibility would be to replace the trailing whitespace(s) by the interpolation: hello # + 1 space
world # + 8 spaces
# corrected to:
hello#{ ' ' }
world#{ ' ' }Is that a better auto-correction? |
|
It seems better/clearer to me. |
marcandre
force-pushed
the
whitespace
branch
from
91ddd7a
to
fe3e2ad
Compare
|
PR updated. I also realized I had to modify |
marcandre
force-pushed
the
whitespace
branch
3 times, most recently
from
ea57399
to
8cd6c7d
Compare
|
Note: I used a Looks like there's a dependency issue though with JRuby... I'll check that tomorrow |
|
This ends up resulting in infinite recursion for us - I'm not sure how to appease the cop, other than just disabling it around heredocs? |
|
I'm sorry @lucywyman, looks like I wrote the right code but somehow later worked from an older version with the empty |
|
#8914 should restore our original intent and fix the issue. Until the next bugfix release, you can either fix them manually, or set your config with: To fix them manually, either remove the trailing whitespace, or replace it with Thanks for reporting this, and again sorry for the trouble. |
|
No prob! Thanks for addressing it so quickly! |
I extracted #8695 to remove all trailing whitespaces in our own codebase.
This PR fixes
Layout/TrailingWhitespaceauto-correction in Heredocs to be safe. Instead of removing the trailing whitespace (which is never equivalent), it inserts#{}after it (which evaluates to an empty string) and thus makes the trailing whitespace no longer trailing. Maybe a more explicit form like#{ '' }or#{ :trailing_whitespace!; '' }would be better though?Given the fact that we can now autocorrect correctly trailing whitespaces in heredoc, and that they can be a pain to deal with for Rubyists like me that set their editor to automatically trim their whitesapce, I feel that the default should be changed in general. I assume that most projects have much less occurrences of trailing whitespace in their heredoc anyways.