Add tests and implementation for a "quotemark" printer option #141
Conversation
|
@weswigham thanks! @benjamn it would be great to see this is in, we are using recast to generate code in https://github.com/ember-cli/ember-cli and people keep complaining that sometimes we generate with double quote even though single quote is our recommendation. |
lib/options.js
Outdated
There was a problem hiding this comment.
Can you change this so that options.quotemark is either null, '"' (rather than "double"), or "'" (rather than "single")?
There was a problem hiding this comment.
I was using "double" and "single" to be inline with the jshint/jslint options, but '"' and "'" are both doable.
There was a problem hiding this comment.
Maybe we could aim to some parity with escodegen? they use the key "quotes" instead of "quotemark" and "single", "double" or "auto" as options. see https://github.com/estools/escodegen/wiki/API#optionformatquotes
There was a problem hiding this comment.
I like mirroring that - though I think auto and null/undefined should be the same?
Edit: I read through what 'auto' does - that's cool, but who actually uses 'whichever literal is smaller'? Is that a use case worth supporting?
|
I removed the over-aggressive literal re-emissions from the patcher, and mirrored the escodegen quote transform api (since, on reflection, selecting the shortest literal makes sense for something like a minifier). Thoughts? |
|
👍 |
|
👍 Thanks |
lib/patcher.js
Outdated
There was a problem hiding this comment.
I might be crazy, but I can't tell where options is used in this file. It seems like it's just being passed through the various find* functions but never examined. Am I missing something obvious?
There was a problem hiding this comment.
You're right! After I removed the overzealous reprinting it wasn't used anymore. Let me clean this up.
|
Made some comments; sorry for the delay! |
|
Removed all the stale changes and changed the whitespace of the switch to your preference. |
Add tests and implementation for a "quotemark" printer option.
|
@weswigham @benjamn thanks! ember-cli/ember-cli#3182 👍 |
4 participants
@sebmarkbage on the React repo mentioned needing Recast to support a feature like this for them to support configurable quotemarks when compiling JSX transformations in the future. (mentioned originally on pull request 1709)