Skip to content

Conversation

@jparise
Copy link
Owner

@jparise jparise commented Aug 6, 2024

Some JavaScript GraphQL frameworks use well-known functions rather than tagged template literals. This change introduces basic support for these cases via a new g:graphql_javascript_functions variable (["graphql"]).

This is implemented as a bit of a hack on top of the existing tagged template support to avoid introducing new syntax regions:

Tagged template:    graphql`
Function call:      graphql(`
Syntax pattern:     (graphql|graphql\()`

This is a good-enough place to start, and the overall approach gives us room to iterate later if we want to revisit the implementation.

Resolves #96

@jparise jparise force-pushed the javascript-functions branch from 77922f8 to afbcfb4 Compare August 6, 2024 12:55
Some JavaScript GraphQL frameworks use well-known functions rather than
tagged template literals. This change introduces basic support for these
cases via a new `g:graphql_javascript_functions` variable (["graphql"]).

This is implemented as a bit of a hack on top of the existing tagged
template support to avoid introducing new syntax regions:

    Tagged template:    graphql`
    Function name:      graphql(`
    Syntax pattern:     (graphql|graphql\()`

This is a good-enough place to start, and the overall approach gives us
room to iterate later if we want to revisit the implementation.
@jparise jparise force-pushed the javascript-functions branch from afbcfb4 to 90667bf Compare August 6, 2024 13:08
@jparise jparise merged commit 9caa247 into master Aug 6, 2024
@jparise jparise deleted the javascript-functions branch August 6, 2024 13:40
@vieira
Copy link

vieira commented Sep 11, 2024

@jparise, thanks for your work on this plugin! I was testing this specific feature and noticed that when using graphql(``) the closing parenthesis seems be highlighted as an error.

I then noticed that in the README and the tests you omit the closing parenthesis and use it like graphql(``; but this does not seem to be valid javascript?

Not sure if this is a bug or if I'm missing something.

jparise added a commit that referenced this pull request Oct 23, 2024
These were missing from #109 but weren't causing any functional
problems.
@jparise
Copy link
Owner Author

jparise commented Oct 23, 2024

@jparise, thanks for your work on this plugin! I was testing this specific feature and noticed that when using graphql() `` the closing parenthesis seems be highlighted as an error.

I'm not able to reproduce this error highlighting (which might be due to us having some configuration or plugin differences), but I'll take another look at the regular expressions to see if there are any improvements I can make. The function-highlighting approach I'm using here is admittedly a bit of a kludge.

I then noticed that in the README and the tests you omit the closing parenthesis and use it like graphql(; `` but this does not seem to be valid javascript?

This was an oversight that I just fixed in f2e7754, but they shouldn't have been causing any functional problems on their own.

@vieira
Copy link

vieira commented Oct 23, 2024

I'm not able to reproduce this error highlighting (which might be due to us having some configuration or plugin differences), but I'll take another look at the regular expressions to see if there are any improvements I can make. The function-highlighting approach I'm using here is admittedly a bit of a kludge.

Thanks for taking the time to look into it. I have just disabled all my plugins except for jparise/vim-graphql and noticed that it stops working, so I realize it must depend on pangloss/vim-javascript so I also enabled it. Both were downloaded from github as a zip from master. The issue persists. Below are some screenshots (with colorscheme default) illustrating the issue. The gql template literal and the gql comments are not affected, only the function call:

Screenshot 2024-10-23 at 17 07 17 Screenshot 2024-10-23 at 17 14 21 Screenshot 2024-10-23 at 17 14 36

Let me know if there is anything I can do to help.

Thanks again.

@jparise
Copy link
Owner Author

jparise commented Oct 28, 2024

Thanks for taking the time to look into it. I have just disabled all my plugins except for jparise/vim-graphql and noticed that it stops working, so I realize it must depend on pangloss/vim-javascript so I also enabled it. Both were downloaded from github as a zip from master. The issue persists. Below are some screenshots (with colorscheme default) illustrating the issue.

Thank you! This is helpful. I don't have a solution yet but it's useful to see what you're seeing. Embedding GraphQL inside of JavaScript has proven to be the most complicated aspect of maintaining this plugin.

Also, we support vim's built-in javascript filetype support and pangloss/vim-javascript. It's interesting that you see different results with each of them.

jparise added a commit that referenced this pull request Apr 27, 2025
Our previous function-matching approach (#109) was implemented on top of
the existing tag-matching patterns. This had the downside of including
the function name and initial parenthesis in the region match.

Instead, use dedicated syntax regions to specifically match the string
template argument passed to our named GraphQL-aware functions.
jparise added a commit that referenced this pull request Apr 27, 2025
Our previous function-matching approach (#109) was implemented on top of
the existing tag-matching patterns. This had the downside of including
the function name and initial parenthesis in the region match.

Instead, use dedicated syntax regions to specifically match the string
template argument passed to our named GraphQL-aware functions for both
JavaScript and TypeScript.
jparise added a commit that referenced this pull request Apr 27, 2025
Our previous function-matching approach (#109) was implemented on top of
the existing tag-matching patterns. This had the downside of including
the function name and initial parenthesis in the region match.

Instead, use dedicated syntax regions to specifically match the string
template argument passed to our named GraphQL-aware functions for both
JavaScript and TypeScript.
@jparise
Copy link
Owner Author

jparise commented Apr 27, 2025

@vieira I'm hopeful that the change I just made (in #111) will improve this for you. Sorry it's taken so long for me to get back into this!

@vieira
Copy link

vieira commented Aug 8, 2025

@vieira I'm hopefully that the change I just made (in #111) will improve this for you. Sorry it's taken so long for me to get back into this!

Hello @jparise. Thanks a lot for the change. I just confirmed it fixes the issue. Sorry for the delay!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Support for magic comments or functions

3 participants