Trim trailing whitespace #5023
Trim trailing whitespace #5023
Conversation
|
Hi @MartyIX, I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution! TTYL, MSBOT; |
| @@ -53,3 +53,4 @@ internal/ | |||
| .settings | |||
| .vscode/* | |||
| !.vscode/tasks.json | |||
| .idea/* | |||
There was a problem hiding this comment.
Is this for WebStorm?
There was a problem hiding this comment.
Yes, it is actually for the family of IDEs from JetBrains - PhpStorm, WebStorm, etc.
|
@MartyIX, Thanks for signing the contribution license agreement so quickly! Actual humans will now validate the agreement and then evaluate the PR. |
|
|
||
| let comments = getLeadingCommentsToEmit(node); | ||
|
|
||
| if (!comments || !comments[0].hasTrailingNewLine) { |
There was a problem hiding this comment.
Could you not just check multiLine for roughly the same emit?
There was a problem hiding this comment.
Maybe @yuit could weigh in on this.
There was a problem hiding this comment.
the variable comments contains data like this [ { pos: 131, end: 177, hasTrailingNewLine: true, kind: 3 } ]. That's why I used hasTrailingNewLine.
|
Hey @MartyIX, I'm not sure about this solution because we don't want to recalculate the leading comments for every list item (which involves a rescan and an array allocation for each node). @yuit and I spent a good bit of time looking into alternative approaches, and while we got something that works decently, the complexity didn't seem worth it. Overall, I think the gain from any sort of fix here is marginal. While I appreciate that you put the time into it, I think I'm going to mark the original issue as |
|
I see |
Proposed fix for #4629