atom / one-light-syntax

Atom One light syntax theme
MIT License
68 stars 72 forks source link

Prevent string styling from bleeding through interpolations #47

Closed maxbrunsfeld closed 6 years ago

maxbrunsfeld commented 6 years ago

Description of the Change

Currently, interpolated code inside of strings is highlighted as a string. Not the green color on the word three below:

interpolation-styling-before

This PR explicitly resets the font color when a scope with the source class appears inside of a string. This works both with the TextMate highlighting and, on Atom master, with Tree-sitter highlighting as well:

interpolation-styling-after

Alternate Designs

We could somehow cause the string scope to actually end at the start of the interpolation and begin again after the interpolation. I think this would be more difficult than what I've done here, with both TextMate and Tree-sitter highlighting.

Benefits

String interpolations are easier to read.

Possible Drawbacks

In some languages, interpolations might not be assigned the source class. It seems reasonable that we could change the grammars to always provide that class though.

Applicable Issues

https://github.com/atom/language-ruby/issues/147

/cc @simurai is @syntax-fg the best variable to use for this purpose?

simurai commented 6 years ago

/cc @simurai is @syntax-fg the best variable to use for this purpose?

@mono-1 could also be used. It's exactly the same as @syntax-fg, but maybe a bit more consistent since the "mono" variables are used in other places of the syntax highlighting.

Anyways, feel free to merge it either way. :smile: Same for https://github.com/atom/one-dark-syntax/pull/112.

In some languages, interpolations might not be assigned the source class.

The TextMate naming_conventions lists: string.interpolated. Could that be used? Although not sure how many themes/languages support it. For example this theme doesn't either.

maxbrunsfeld commented 6 years ago

The TextMate naming_conventions lists: string.interpolated

Yeah, that would make sense. Unfortunately, the existing grammars don't seem to use those scopes. But this suggestion did prompt me to double check what scopes they all do use, and I realized that they don't all use the source scope here either. Ruby and JavaScript do, but Python and TypeScript don't. They do however all use the meta.embedded scope. So I think I'll update the PR to target that scope.

simurai commented 6 years ago

They do however all use the meta.embedded scope.

Ha.. interesting. Just in case, we can always style certain languages separately if they don't follow a convention that most others use.