From 287db4a9b6f091d665421857977a36e770e7cc07 Mon Sep 17 00:00:00 2001 From: Christopher Chedeau Date: Mon, 13 Feb 2017 07:12:31 -0800 Subject: [PATCH] Stabilize comments inside of ternaries (#666) Now, all the variations of comments in ternaries are printed the same way. Fixes #617 --- src/comments.js | 16 ++++- .../__snapshots__/jsfmt.spec.js.snap | 71 +++++++++++++++++++ tests/conditional/comments.js | 33 +++++++++ tests/conditional/jsfmt.spec.js | 1 + 4 files changed, 118 insertions(+), 3 deletions(-) create mode 100644 tests/conditional/__snapshots__/jsfmt.spec.js.snap create mode 100644 tests/conditional/comments.js create mode 100644 tests/conditional/jsfmt.spec.js diff --git a/src/comments.js b/src/comments.js index 6109ce83..695326cd 100644 --- a/src/comments.js +++ b/src/comments.js @@ -157,9 +157,11 @@ function attach(comments, ast, text) { addDanglingComment(ast, comment); } } else if (util.hasNewline(text, locEnd(comment))) { - // There is content before this comment on the same line, but - // none after it, so prefer a trailing comment of the previous node. - if (precedingNode) { + if (handleConditionalExpressionComments(enclosingNode, followingNode, comment)) { + // We're good + } else if (precedingNode) { + // There is content before this comment on the same line, but + // none after it, so prefer a trailing comment of the previous node. addTrailingComment(precedingNode, comment); } else if (followingNode) { addLeadingComment(followingNode, comment); @@ -369,6 +371,14 @@ function handleMemberExpressionComment(enclosingNode, followingNode, comment) { return false; } +function handleConditionalExpressionComments(enclosingNode, followingNode, comment) { + if (enclosingNode && enclosingNode.type === 'ConditionalExpression' && followingNode) { + addLeadingComment(followingNode, comment); + return true; + } + return false; +} + function printComment(commentPath) { const comment = commentPath.getValue(); diff --git a/tests/conditional/__snapshots__/jsfmt.spec.js.snap b/tests/conditional/__snapshots__/jsfmt.spec.js.snap new file mode 100644 index 00000000..688b1095 --- /dev/null +++ b/tests/conditional/__snapshots__/jsfmt.spec.js.snap @@ -0,0 +1,71 @@ +exports[`test comments.js 1`] = ` +"var inspect = 4 === util.inspect.length + ? // node <= 0.8.x + (function(v, colors) { + return util.inspect(v, void 0, void 0, colors); + }) + : // node > 0.8.x + (function(v, colors) { + return util.inspect(v, { colors: colors }); + }); + +var inspect = 4 === util.inspect.length + ? // node <= 0.8.x + (function(v, colors) { + return util.inspect(v, void 0, void 0, colors); + }) + : // node > 0.8.x + (function(v, colors) { + return util.inspect(v, { colors: colors }); + }); + +const extractTextPluginOptions = shouldUseRelativeAssetPaths + // Making sure that the publicPath goes back to to build folder. + ? { publicPath: Array(cssFilename.split(\'/\').length).join(\'../\') } : + {}; + +const extractTextPluginOptions = shouldUseRelativeAssetPaths + ? // Making sure that the publicPath goes back to to build folder. + { publicPath: Array(cssFilename.split(\"/\").length).join(\"../\") } + : {}; + +const extractTextPluginOptions = shouldUseRelativeAssetPaths // Making sure that the publicPath goes back to to build folder. + ? { publicPath: Array(cssFilename.split(\"/\").length).join(\"../\") } + : {}; +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +var inspect = 4 === util.inspect.length + ? // node <= 0.8.x + (function(v, colors) { + return util.inspect(v, void 0, void 0, colors); + }) + : // node > 0.8.x + (function(v, colors) { + return util.inspect(v, { colors: colors }); + }); + +var inspect = 4 === util.inspect.length + ? // node <= 0.8.x + (function(v, colors) { + return util.inspect(v, void 0, void 0, colors); + }) + : // node > 0.8.x + (function(v, colors) { + return util.inspect(v, { colors: colors }); + }); + +const extractTextPluginOptions = shouldUseRelativeAssetPaths + ? // Making sure that the publicPath goes back to to build folder. + { publicPath: Array(cssFilename.split(\"/\").length).join(\"../\") } + : {}; + +const extractTextPluginOptions = shouldUseRelativeAssetPaths + ? // Making sure that the publicPath goes back to to build folder. + { publicPath: Array(cssFilename.split(\"/\").length).join(\"../\") } + : {}; + +const extractTextPluginOptions = shouldUseRelativeAssetPaths + ? // Making sure that the publicPath goes back to to build folder. + { publicPath: Array(cssFilename.split(\"/\").length).join(\"../\") } + : {}; +" +`; diff --git a/tests/conditional/comments.js b/tests/conditional/comments.js new file mode 100644 index 00000000..78e9a024 --- /dev/null +++ b/tests/conditional/comments.js @@ -0,0 +1,33 @@ +var inspect = 4 === util.inspect.length + ? // node <= 0.8.x + (function(v, colors) { + return util.inspect(v, void 0, void 0, colors); + }) + : // node > 0.8.x + (function(v, colors) { + return util.inspect(v, { colors: colors }); + }); + +var inspect = 4 === util.inspect.length + ? // node <= 0.8.x + (function(v, colors) { + return util.inspect(v, void 0, void 0, colors); + }) + : // node > 0.8.x + (function(v, colors) { + return util.inspect(v, { colors: colors }); + }); + +const extractTextPluginOptions = shouldUseRelativeAssetPaths + // Making sure that the publicPath goes back to to build folder. + ? { publicPath: Array(cssFilename.split('/').length).join('../') } : + {}; + +const extractTextPluginOptions = shouldUseRelativeAssetPaths + ? // Making sure that the publicPath goes back to to build folder. + { publicPath: Array(cssFilename.split("/").length).join("../") } + : {}; + +const extractTextPluginOptions = shouldUseRelativeAssetPaths // Making sure that the publicPath goes back to to build folder. + ? { publicPath: Array(cssFilename.split("/").length).join("../") } + : {}; diff --git a/tests/conditional/jsfmt.spec.js b/tests/conditional/jsfmt.spec.js new file mode 100644 index 00000000..989047bc --- /dev/null +++ b/tests/conditional/jsfmt.spec.js @@ -0,0 +1 @@ +run_spec(__dirname);