Allow formatting ranges of JSON #2298
Conversation
index.js
Outdated
| @@ -195,6 +202,12 @@ function isSourceElement(node) { | |||
| case "TypeAliasDeclaration": // Typescript | |||
| case "ExportAssignment": // Typescript | |||
| case "ExportDeclaration": // Typescript | |||
| case "ObjectExpression": // JSON | |||
There was a problem hiding this comment.
Can you only do that when you're in a json context? Otherwise it's going to format weird things inside of js.
There was a problem hiding this comment.
It would be great to have each parser with its own list IMOH
There was a problem hiding this comment.
Good point, fixed in 2963104
We can fix the other parsers separately.
This is so that the AST_COMPARE=1 tests don't try to parse JSON with Flow.
Otherwise JSON formatting would format weird things inside of JS. See prettier#2298 (comment)
index.js
Outdated
| @@ -235,12 +236,12 @@ function calculateRange(text, opts, ast) { | |||
| const startNodeAndParents = findNodeAtOffset( | |||
| ast, | |||
| startNonWhitespace, | |||
| isSourceElement | |||
| isSourceElement.bind(null, opts) | |||
There was a problem hiding this comment.
can you use arrow functions, I find them easier to understand
node => isSourceElement(opts, node)There was a problem hiding this comment.
Sure, fixed in e06dc79
index.js
Outdated
| case "NumericLiteral": // JSON | ||
| case "BooleanLiteral": // JSON | ||
| case "NullLiteral": // JSON | ||
| return opts.parser === "json"; |
There was a problem hiding this comment.
Are those all the possible node types? Should we just return true if using the json parser?
There was a problem hiding this comment.
No, there's also ObjectProperty and json-identifier. I guess the identifier is technically formattable, so maybe it should be included too.
EDIT: Fixed in 12324ef
lock
bot
added
the
locked-due-to-inactivity
This fixes #2297