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
fix: Fix vscode-graphql-syntax’s grammar to support string literals on separate lines [Reapply & Fix] #3545
fix: Fix vscode-graphql-syntax’s grammar to support string literals on separate lines [Reapply & Fix] #3545
Conversation
🦋 Changeset detectedLatest commit: 1bd052b The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Zip containing a |
thank you @kitten !!! looks awesome! |
packages/vscode-graphql-syntax/tests/__snapshots__/markdown-grammar.spec.ts.snap
Outdated
Show resolved
Hide resolved
…n separate lines (graphql#3518) * Allow newlines after `graphql(` before start of query string * Fix mistaken reverse order of optional `(` and generic `<.*>` * Replace `(` in second match with newline one * Match graphql calls separately for string literals inside calls * Add missing endCaptures to new rule
Using a positive lookbehind can subtly break Textmate/VSCode’s syntax highlighting. The positive lookbehind (according to some online sources) can fail unexpectedly when they contain whitespace matches. This means that we should instead match explicitly, like the other patterns do. This means however, that we might match too much and I noticed that arguments to the function aren't highlighted correctly. According to the interpolation rule in `graphql.json` we now include patterns for JS/TS/etc as a fallback pattern. This fixes the issue and prevents the case where the rule was conflicting with normal JS/TS patterns in a different tmLanguage grammar.
562499c
to
1bd052b
Compare
Rebased & reverted changes to |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok let's do this again!
If there are any more edge case bugs this time, instead of reverting we will create new tests and solve the issues as they arise
Resolves #3542
Reapplies: #3518
Related: #3543
The previous implementation for GraphQL template string literals inside function expression caused a subtle error in syntax highlighting.
Because it used a positive lookbehind that contained whitespace matches, this caused an issue with syntax matching, which breaks subsequent TS/JS rules from matching. In other words, it caused a conflict between built-in highlighting and the embedded GraphQL highlighting.
Instead of this, we now match the function expressions explicitly, and include JS/TS rules inside the match to also fix arguments that live alongside the GraphQL template string.