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

Use injections for SQL strings #330

Merged
merged 2 commits into from May 19, 2018

Conversation

Projects
None yet
2 participants
@50Wliu
Member

50Wliu commented May 16, 2018

Requirements

  • Filling out the template is required. Any pull request that does not include enough information to be reviewed in a timely manner may be closed at the maintainers' discretion.
  • All new code requires tests to ensure against regressions

Description of the Change

Now that language-sql has begin/end matches, injections are needed so that the PHP patterns are matched inside the begin/end ones as well. Unfortunately, due to some annoyances with how injections work, I had to place the injections inside of the wrapper html.cson rather than php.cson, which would make much more sense.

I am also having trouble getting interpolation to work properly. Seems like it's not picking up source.php correctly, which is discouraging...

Alternate Designs

I would love to hear proposals for alternate designs.

Benefits

Embedded SQL strings should no longer have the chance of ruining syntax highlighting for the rest of the file.

Possible Drawbacks

Grammar becomes harder to parse due to the injections being in a separate file.

Applicable Issues

Fixes #321

@50Wliu

This comment has been minimized.

Member

50Wliu commented May 16, 2018

/cc @Ingramz it would be super appreciated if you could take a look at this and see if you can figure out why the interpolation includes aren't working.

@Ingramz

This comment has been minimized.

Contributor

Ingramz commented May 16, 2018

@50Wliu I took a quick look at it and it seems like the culprit is that you are trying to include from a repository of a different grammar inside the injection: source.php#interpolation_double_quoted.

When I copied these repository items to the html grammar's repository and included those instead, it seemed to work without issues. Indirectly including source.php#interpolation_double_quoted from local repository within the injection did not work either.

Whether it is supposed to work in TextMate remains to be seen once I get home from work.

Overall, the idea of using injections seems suitable for at least PHP. Having to do it from the html grammar is somewhat suboptimal, but since nested injections definitely do not work in TextMate, this is probably the best chance we got if it works.

@Ingramz

This comment has been minimized.

Contributor

Ingramz commented May 16, 2018

My tests with TextMate show that including from other grammar's repository should work in injections too, so it is likely that something needs to be fixed in first-mate to get this to work.

@50Wliu 50Wliu changed the title from [WIP] Use injections for SQL strings to Use injections for SQL strings May 19, 2018

@50Wliu 50Wliu merged commit 578fe7e into master May 19, 2018

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@50Wliu 50Wliu deleted the wl-sql-injections branch May 19, 2018

@50Wliu 50Wliu referenced this pull request May 25, 2018

Closed

MySQL syntax highlight breaks within parentheses () #331

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