Skip to content
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

Support toplevel interpolations in graphql tags #3370

Merged
merged 4 commits into from Dec 3, 2017

Conversation

lydell
Copy link
Member

@lydell lydell commented Dec 1, 2017

Fixes #3318.


# comment
\${test} # comment
\`
Copy link
Member Author

@lydell lydell Dec 1, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All the comments after the interpolations here are currently moved to the next line. Is this a deal-breaker? This seems to require some larger changes to made.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably not a deal-breaker. I'm guessing comments are pretty uncommon here. I've only ever used comments in GraphQL schemas.

@azz
Copy link
Member

azz commented Dec 1, 2017

Multiparser refactor looks good. Haven't reviewed GraphQL changes properly yet but test outputs look good.

@lydell
Copy link
Member Author

lydell commented Dec 1, 2017

I need to figure out why the parentParser thing doesn’t work, though.

}
}

function fromMarkdown(path, options) {
function parseAndPrint(passedOptions) {
Copy link
Member

@duailibe duailibe Dec 1, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You'd need to pass the previous and next options here so the parentParser thing works.. see printSubtree how it builds next.options but gets the parentParser from options

Right now, you have parser === parentParser

@lydell lydell changed the title [WIP] Support toplevel interpolations in graphql tags Support toplevel interpolations in graphql tags Dec 1, 2017
@lydell
Copy link
Member Author

lydell commented Dec 1, 2017

I think I'm happy with this now. Would be awesome if it could make it into the 1.9 release!

@duailibe duailibe added this to the 1.9 milestone Dec 1, 2017
@lydell
Copy link
Member Author

lydell commented Dec 2, 2017

Haven't reviewed GraphQL changes properly yet

@azz Do you want to take a look before merging, or should I just press the green button?

@azz azz merged commit d80728b into prettier:master Dec 3, 2017
@lydell lydell deleted the gql-interpolations branch December 3, 2017 08:42
@lock lock bot added the locked-due-to-inactivity Please open a new issue and fill out the template instead of commenting. label Jan 19, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Jan 19, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
locked-due-to-inactivity Please open a new issue and fill out the template instead of commenting.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants