Skip to content
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

An embedded syntax always has its main scope added #2482

Closed
FichteFoll opened this issue Nov 18, 2018 · 10 comments
Closed

An embedded syntax always has its main scope added #2482

FichteFoll opened this issue Nov 18, 2018 · 10 comments

Comments

@FichteFoll
Copy link
Collaborator

When embedding a scope/syntax and using embed_scope to set a different scope for the embed, the embed's main scope is still added to the scope path. This results in duplicated scope names when setting an embed scope like source.css.embedded.html.

Example from the default HTML.sublime-syntax:

  style-css:
    - meta_content_scope: meta.tag.style.begin.html
    - include: style-common
    - match: '>'
      scope: punctuation.definition.tag.end.html
      set:
        - include: style-close-tag
        - match: ''
          embed: scope:source.css
          embed_scope: source.css.embedded.html
          escape: (?i)(?=(?:-->\s*)?</style)

The resulting scope in a <style> tag is text.html.basic source.css.embedded.html source.css.

I propose to add another key that control's whether the pushed, set or embedded context's meta scope is added to the scope path to prevent this.

@keith-hall
Copy link
Collaborator

keith-hall commented Nov 22, 2018

I like this idea, because the alternative is to create a new named context that just includes the desired syntax definition, so that the base scope isn't applied. (It currently isn't possible to embed an anonymous context, and as ST is sensitive to the number of contexts a definition has, it still wouldn't be preferable even if it were possible.)

BTW, the behavior of including the base scope seems to mainly be related to how ST resolves context references that refer to the main context of a (potentially) foreign syntax definition. i.e. embedding/pushing/setting main will never include the base/root scope, but scope:source.css, scope:source.css#main, CSS.sublime-syntax and CSS.sublime-syntax#main will all exhibit this behavior, so I'm not sure how easy it will be for sublimehq to add a key for this.

@michaelblyons
Copy link

Notice that the PHP syntax definition uses embedding.php as its root scope, presumably for this reason. I was tinkering with one of my own this week because of this section. It results in color schemes targeting source source matching the whole file.

/backlink

@wbond
Copy link
Member

wbond commented Jun 6, 2020

As a note: this is a side effect of compatibility with TextMate - when referencing a context in an external syntax, the main context of the external syntax always has the external syntax's scope added to the content scope. This is true if you push or embed.

@deathaxe
Copy link
Collaborator

deathaxe commented Jun 6, 2020

A syntax definition has already indirect control about whether the main scope of an embedded syntax is added or not.

Not sure whether it is a desirable solution or workaround though.

In order to workaround #1062 a syntax needs to include both the main and the prototype context of the embedded syntax in order to prevent the embedded syntax from being inproperly highlighted due to missing rules which are present in prototype only. To do so an intermediate context is required.

As a side effect of this workaround the main scope of the embedded syntax is omitted.

The solution with regards to the example of HTML.sublime-syntax looks like:

  style-css:
    - meta_content_scope: meta.tag.style.begin.html
    - include: style-common
    - match: '>'
      scope: punctuation.definition.tag.end.html
      set:
        - include: style-close-tag
        - match: (?=\S)
          embed: style-css-embedded
          embed_scope: source.css.embedded.html
          escape: (?i)(?=(?:-->\s*)?</style)

  style-css-embedded:
    # workaround for https://github.com/sublimehq/sublime_text/issues/1062
    - include: scope:source.css#prototype
    - include: scope:source.css

The downside of this solution is that we literly never can embed a scope:... directly but always need an additional context like style-css-embedded which includes the 3rd party syntax.

@wbond
Copy link
Member

wbond commented Jun 26, 2020

Edit: sorry, wrong issue.

I was getting different results (prototype was never applied to any external syntax).

Turns out the issue is that the order of processing the contexts affects things, at least in the case of using include.

@wbond
Copy link
Member

wbond commented Jun 29, 2020

Typically we use embed to include another syntax via the root scope name, e.g. for CSS scope:source.css.

Because of this, we would know the scope of the syntax that we are embedding. Alternatively, if we are specifying the external syntax via a file name, we are presumably targeting a syntax we have control over.

As a result, I propose that a solution would be to skip the external syntax scope name, if embed_scope is specified.

If the user wants to include the original scope and a new one, they specify both. That shouldn't be brittle since we are normally specifying the external syntax by scope anyway.

Furthermore, embed is a relatively rare feature, and so documenting and requiring such a change for "version 2" syntaxes would not be much of a maintenance burden. Especially when the other backwards compatibility changes are scope related, and rather a bit more common.

Thoughts?

@keith-hall
Copy link
Collaborator

Sounds sensible to me :)

@deathaxe
Copy link
Collaborator

As a result, I propose that a solution would be to skip the external syntax scope name, if embed_scope is specified.

Sounds great and straight forward. Avoids the need of intermediate contexts just for meta scope handling. Don't need to abuse - include then.

@wbond
Copy link
Member

wbond commented Jun 29, 2020

Unfortunately the implementation of this change looks to be extremely invasive.

@wbond wbond self-assigned this Jun 30, 2020
@wbond
Copy link
Member

wbond commented Jul 10, 2020

As of build 4075, if a .sublime-syntax has version: 2 and specifies embed_scope, the "main" context of an external syntax will not have the syntax scope prepended.

@wbond wbond closed this as completed Jul 10, 2020
@wbond wbond added the R: fixed label Jul 10, 2020
@wbond wbond added this to the Build 4075 milestone Jul 10, 2020
@wbond wbond removed their assignment Aug 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants