[RFC] Fix comment location for binary expressions (#1043)

The root cause is that we're calling `printComments` with an empty string, meaning that the leading/trailing comments are not correctly inserted at the right location.

So, the way to fix it is to call `p => concat(parts)` but because we're mutating the array in place, it doesn't work. We need to mutate it and create a copy. But, the root call is actually checking the shape of the parts array which our code is now breaking...

```js
          // Don't include the initial expression in the indentation
          // level. The first item is guaranteed to be the first
          // left-most expression.
          parts.length > 0 ? parts[0] : "",
```

The consequence is that binary expressions are no longer indented if the first expression has a comment (but now it places the comment properly!), which seems like a good trade-off.

I'm not sure if we should merge this one or instead refactor this code such that it doesn't rely on mutation and looking at the shape of the printed tree. `printMemberChain` is a good thing to reference for inspiration.

Fixes #946
master
Christopher Chedeau 2017-03-21 12:13:13 -07:00 committed by GitHub
parent 24f9cc3a71
commit ce6e897950
4 changed files with 33 additions and 9 deletions

View File

@ -3013,7 +3013,11 @@ function printBinaryishExpressions(path, parts, print, options, isNested) {
// the other ones since we don't call the normal print on BinaryExpression,
// only for the left and right parts
if (isNested && node.comments) {
parts.push(comments.printComments(path, p => "", options));
parts.splice(
0,
parts.length,
comments.printComments(path, p => concat(parts.slice()), options)
);
}
} else {
// Our stopping case. Simply print the node normally.

View File

@ -19,6 +19,21 @@ function f() {
"
`;
exports[`comment.js 1`] = `
"a = (
// Commment 1
(Math.random() * (yRange * (1 - minVerticalFraction)))
+ (minVerticalFraction * yRange)
) - offset;
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
a =
// Commment 1
Math.random() * (yRange * (1 - minVerticalFraction)) +
minVerticalFraction * yRange -
offset;
"
`;
exports[`exp.js 1`] = `
"a ** b ** c;
(a ** b) ** c;

View File

@ -0,0 +1,5 @@
a = (
// Commment 1
(Math.random() * (yRange * (1 - minVerticalFraction)))
+ (minVerticalFraction * yRange)
) - offset;

View File

@ -712,10 +712,10 @@ export type AsyncExecuteOptions = child_process$execFileOpts & {
// optional trailing comma gets moved all the way to the beginning
const regex = new RegExp(
\\"^\\\\\\\\s*\\" + // beginning of the line
\\"name\\\\\\\\s*=\\\\\\\\s*\\" + // name =
\\"['\\\\\\"]\\" + // opening quotation mark
escapeStringRegExp(target.name) + // target name
\\"['\\\\\\"]\\" + // closing quotation mark
\\"name\\\\\\\\s*=\\\\\\\\s*\\" + // name =
\\"['\\\\\\"]\\" + // opening quotation mark
escapeStringRegExp(target.name) + // target name
\\"['\\\\\\"]\\" + // closing quotation mark
\\",?$\\" // optional trailing comma
);
@ -938,10 +938,10 @@ export type AsyncExecuteOptions = child_process$execFileOpts & {
// optional trailing comma gets moved all the way to the beginning
const regex = new RegExp(
\\"^\\\\\\\\s*\\" + // beginning of the line
\\"name\\\\\\\\s*=\\\\\\\\s*\\" + // name =
\\"['\\\\\\"]\\" + // opening quotation mark
escapeStringRegExp(target.name) + // target name
\\"['\\\\\\"]\\" + // closing quotation mark
\\"name\\\\\\\\s*=\\\\\\\\s*\\" + // name =
\\"['\\\\\\"]\\" + // opening quotation mark
escapeStringRegExp(target.name) + // target name
\\"['\\\\\\"]\\" + // closing quotation mark
\\",?$\\" // optional trailing comma
);