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

Add support for code embedded in strings #294

Merged
merged 1 commit into from Jul 14, 2017

Conversation

Projects
None yet
2 participants
@MoritzKn
Contributor

MoritzKn commented Jun 21, 2017

This PR will implement #274. This means it will allow bracket matching for embedded code like this:

`${(15 + 5) * 5}`
   ^      ^

The ^ marks the highlighting.

Description of the Change

The BracketMatcherView detects if the current bracket is inside a comment or a string and in this case ignores it completely. In order to allow embedded code I changed the why this happens. Previously a ScopeSelector was used. I replaced the selector with a manual check.

Alternate Designs

I first tried implementing this by just changing the commentOrStringSelector selector. But was unable to do so. As far as I can tell this would require a negative lookahead and I couldn't find anything like this for scope selectors.

Benefits

Add the enhancement described in #274. Perhaps it even fixes a regression introduced by #253, but that's a matter of interpretation.

Possible Drawbacks

Code like this will match like so:

1 foo = (`
2       ^
3 ${ )}
4    ^
5 `)

However I think that is not much of a problem since the single braked in line 4 would be a syntax error.

Applicable Issues

#274

Additional Information

In #274 @schlomo pointed out that this is also a problem for shell code (comment).
However because the language-shellscript package uses the scope string.interpolated.dollar.shell for code embedded via$() and this PR checks for the scope embedded.source.* this PR won't fix the issue for shell code. I think this is an issue with the language package but I could also add an alias for this scope.

@50Wliu

This comment has been minimized.

Show comment
Hide comment
@50Wliu

50Wliu Jul 13, 2017

Member

Thanks! Can you rebase this on master so I can verify that Travis passes?

Member

50Wliu commented Jul 13, 2017

Thanks! Can you rebase this on master so I can verify that Travis passes?

Add support for code embedded in strings
This allows bracket matching in code link this:
`${(15 + 5) * 5}`
   ^      ^

This will fix #274
@MoritzKn

This comment has been minimized.

Show comment
Hide comment
@MoritzKn

MoritzKn Jul 14, 2017

Contributor

@50Wliu sure thing, it's rebased.

Contributor

MoritzKn commented Jul 14, 2017

@50Wliu sure thing, it's rebased.

@50Wliu 50Wliu merged commit 5c6319c into atom:master Jul 14, 2017

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment