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
Handle closure compiler type cast syntax correctly #2484
Conversation
src/comments.js
Outdated
@@ -990,6 +995,12 @@ function printComments(path, print, options, needsSemi) { | |||
const text = options.originalText; | |||
if (util.hasNewline(text, util.skipNewline(text, util.locEnd(comment)))) { | |||
leadingParts.push(hardline); | |||
} else if (util.isClosureCompilerTypeCastComment(text, comment)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm wondering if this can instead be handled in fast-path.js
's needsParens
function.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed. Could you look into it? If that works that would be cleaner.
Any idea how this will play with flow comment types? |
src/util.js
Outdated
// Syntax example: var x = /** @type {string} */ (fruit); | ||
return ( | ||
isBlockComment(comment) && | ||
comment.value.match(/^\*\s+@type\s+{[^}]+}\s+$/) && |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
probably want to use \s*
instead of \s+
and add some inside of {}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oops. that's what i meant to use.
Thanks for working on it! |
@azz I tested a few variations of the flow comment type syntax and it seems that the changes I made to how comments are attached behave correctly. |
src/util.js
Outdated
// Syntax example: var x = /** @type {string} */ (fruit); | ||
return ( | ||
node.comments && | ||
node.comments.every( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be every
or some
?
Codecov Report
@@ Coverage Diff @@
## master #2484 +/- ##
=========================================
Coverage ? 94.98%
=========================================
Files ? 24
Lines ? 3291
Branches ? 867
=========================================
Hits ? 3126
Misses ? 150
Partials ? 15
Continue to review full report at Codecov.
|
@azz didn't you disable codecov comments? |
@vjeux I certainly did... Maybe needs rebase? |
0c592ab
to
f5b43ae
Compare
f5b43ae
to
bbbb1cf
Compare
Heh, your comment appeared right as I clicked the merge button. |
Fixes #1445