Add a leading space on binary operator token trailing comments#17691
Conversation
sandersn
left a comment
There was a problem hiding this comment.
Looks good. The comment you added seems wrong though.
| emitLeadingCommentsOfPosition(node.operatorToken.pos); | ||
| writeTokenNode(node.operatorToken); | ||
| emitTrailingCommentsOfPosition(node.operatorToken.end); | ||
| emitTrailingCommentsOfPosition(node.operatorToken.end, /*prefixSpace*/ true); // Comma should have a space after it |
There was a problem hiding this comment.
Comma? This is true for all binary operators, right?
There was a problem hiding this comment.
Yep, that comment's a bit too specific. I'll correct it.
| var c = 'some' | ||
| /* comment */ | ||
| +/*comment1*/ | ||
| + /*comment1*/ |
There was a problem hiding this comment.
This output matches the input now.
There was a problem hiding this comment.
And no longer has trailing whitespace.
| 1 | ||
| // before | ||
| >>>// after | ||
| >>> // after |
There was a problem hiding this comment.
We shouldn't emit a space if the comment has a leading newline
There was a problem hiding this comment.
This comment doesn't have a leading newline, though? And the output for this test now exactly matches the input.
|
@rbuckton In all cases, both in our test suite and in our RWC suite, just having there be a space before the trailing comment of a binary operator instead of after always matches the original comment spacing-presence. It's likely just because not many people write code like |
|
You are correct. GitHub's diff didn't give enough apparent context and the |
…soft#17691) * Add a leading space on token trailing comments * Demystify comment
Looking through our RWC results, I realized that we could be a bit better in how we emitted comments on binary expressions (from #16584), thanks to work done in #17557.