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

The prototype context is not included when specifying a syntax file #1062

Closed
FichteFoll opened this issue Jan 13, 2016 · 18 comments
Closed

The prototype context is not included when specifying a syntax file #1062

FichteFoll opened this issue Jan 13, 2016 · 18 comments

Comments

@FichteFoll
Copy link
Collaborator

Specifying an include like so:

    - include: Packages/YAML 1.2/YAML 1.2.sublime-syntax

seems to ignore prototype matches for the 'main' context. Sub-contexts seem to have the syntax's prototype applied however. This is inconsistent.

In order to produce the desired result, one has to include the syntax and additionally the syntax's prototype context like so:

    - include: Packages/YAML 1.2/YAML 1.2.sublime-syntax#prototype
    - include: Packages/YAML 1.2/YAML 1.2.sublime-syntax
@djspiewak
Copy link

This affects github-flavored markdown modes quite significantly, since most language modes use prototype to handle comments.

@wbond
Copy link
Member

wbond commented Sep 19, 2016

As a temporary work-around, explicitly include the prototype scopes in main.

@rwols
Copy link

rwols commented Jan 13, 2018

This also affects the embed action.

deathaxe added a commit to SublimeText/AFileIcon that referenced this issue Feb 8, 2020
Fixes #21

Due to sublimehq/sublime_text#1062
the `prototype` context of a super syntax needs to be included into
all alias syntaxes.
deathaxe added a commit to deathaxe/sublime-packages that referenced this issue Apr 1, 2020
Fixes sublimehq#2318

Problem Description

  The Makefile.sublime-syntax includes the Bash.sublime-syntax.

  The `scope:source.shell#prototype` is injected into the embedded
  Bash.sublime-syntax using `with_prototype` maybe as workaround for
  sublimehq/sublime_text#1062

  The `with_prototype` command ignores `meta_include_prototype: false`,
  which is heavily used by Bash.sublime-syntax to prevent the
  `prototype` context from being included into a bunch of contexts such
  as strings.

Fix Description

  As the prototype context is not included into source.shell's main
  context only, it is not necessary to inject it via `with_protoype`. Hence it is moved to the `shell-body` together with `source.shell`,
  which is suggested by the core issue as proper workaround.

Notes:

  The `prototype` context of the Bash.sublime-syntax also includes
  proper rules to pop off all contexts at eol with respect of line
  continuation. Hence those don't need to be part of the `with_proptype`
  statement as well. They are moved to just pop off the Bash's main
  context as soon as the eol is reached.

  The `source.shell.embedded` scope is applied to all embedded shell
  syntax regions.
@deathaxe
Copy link
Collaborator

deathaxe commented Jun 6, 2020

If a Syntax B includes a single context functions from a Syntax A the prototype of Syntax A is not applied as well. The behavior described in this issue is therefore not limited to the main context.

Example

Syntax A

%YAML 1.2
---

name: Syntax A
scope: source.syntax-a

contexts:
  main:
    - include: functions

  prototype:
    - match: \d
      scope: constant.numeric.integer.syntax-a

  functions:
    - match: \w+
      scope: entity.name.function.syntax-a

Syntax B

%YAML 1.2
---

name: Syntax B
scope: source.syntax-b

contexts:
  main:
    - include: scope:source.syntax-a#functions

If the following is typed to a new view, the number is not highlighted as constant if Syntax B is applied.

10

function

This behavior makes absolutly sense as it is not obvious what happens with prototype being included implicitly as well, when picking single contexts. With a look at Bash.sublime-syntax and the way it cooperates with the auto generated commands-builtin-shell-bash.sublime-syntax any other behavior could easily result in inclusion loops, maybe.


With that in mind I wonder if it was a reasonable solution to

  1. implicitly import a syntaxes prototype if it is included/pushed/set/embedded by another one via scope:<main-scope-identifier> (e.g.: scope:source.c)
  2. not import the prototype if context is included/pushed/set/embedded explicitly by
    scope:<main-scope-identifier>#<context> (e.g.: scope:source.c#main)

Example

%YAML 1.2
---

name: My Syntax
scope: source.my-syntax

contexts:
  main:
    - include: with_prototype
    - include: without_prototype

  with_prototype:
    # include prototype of `source.c`
    - include: scope:source.c

    # include prototype of `source.c`
    - match: blabla
      push: 
        - include: scope:source.c

    # include prototype of `source.c`
    - match: blabla
      push: scope:source.c

    # include prototype of `source.c`
    - match: blabla
      set: scope:source.c

    # include prototype of `source.c`
    - match: blabla
      embed: scope:source.c
      escape: ;

  without_prototype:
    # do not include prototype of `source.c`
    - include: scope:source.c#main

    # do not include prototype of `source.c`
    - match: blabla
      push: scope:source.c#main

    # do not include prototype of `source.c`
    - match: blabla
      set: scope:source.c#main

    # do not include prototype of `source.c`
    - match: blabla
      embed: scope:source.c#main
      escape: ;

With regards to #2482 (comment) it would propably even make sense to limit the implicit include to direct push/set statements and embeds.

Note: Find it less consistent compared with the previous solution though.

Example

%YAML 1.2
---

name: My Syntax
scope: source.my-syntax

contexts:
  main:
    # do not include prototype of `source.c`
    - include: scope:source.c

    # do not include prototype of `source.c`
    - match: blabla
      push: 
        - include: scope:source.c

    # include prototype of `source.c`
    - match: blabla
      push: scope:source.c

    # include prototype of `source.c`
    - match: blabla
      set: scope:source.c

    # include prototype of `source.c`
    - match: blabla
      embed: scope:source.c
      escape: ;

@FichteFoll
Copy link
Collaborator Author

@deathaxe I think you forgot to update a few comments in the without_prototype of your first example.

Other than that, I support the proposal to only include the prototype for includes/pushes where no context is provided. If the prototype was undesired the syntax's #main can be referenced instead.

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

wbond commented Jun 26, 2020

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.

@deathaxe
Copy link
Collaborator

prototype was never applied to any external syntax

It reads as the expectation was the prototype context of the including syntax to be applied to the included one.

Just to be sure - that's not the case.

Hence, your result is not different from what we see, IMHO.

To clarify: My recent comment is only about possibilities to give a syntax control about whether to include a syntax with or without applying its prototype. May not be sufficient though. - It is not to demonstrate any actual behavior.

I guess it is not expected scope:source.foo#prototype to be applied to scope:source.foo#any-context if the latter one is included into a syntax. If we include certain contexts, we might want to access their rules only (without prototypes). W are able to manually include the corresponding prototype if needed. We just would need to be sure ST doesn't throw an error if the included syntax doesn't provide a prototype. I have no strong oppinion about whether prototype should be applied in such situations or not. There may be situations it is desireble and those it would not, propably.

The main point is: if a whole syntax is included using - include: scope:source.foo or embed: scope:source.foo it should behave as if a file was opened using scope:source.foo directly. This requires scope:source.foo#prototype to be applied. Otherwise the included syntax may be broken when being embedded (e.g.: missing comments etc.).

There are two workarounds for that (nearly) out in the wild:

  1. Include prototype into the main context of a syntax (see: Java).

    main:
      - include: prototype
    
  2. Others include the prototype in the including syntax (see: [Makefile] Fix embedded ShellScript prototypes Packages#2320)

    anycontext:
      - include: source.foo#prototype
      - include: source.foo
    

They might conflict or create avoidable overhead if both the included and including syntax try to fix it. The situation is just not clear.

@wbond
Copy link
Member

wbond commented Jun 27, 2020

I spent some more time thinking about this, and I think the current behavior is more or less correct, with the possibility of one change.

In short, including a context should generally never include the prototype automatically. If you are in a syntax and include a support_a context into expressions you would not expect that prototype and then support_a would be included, right? If you wanted prototype in expressions, it would happen automatically.

Now, when dealing with external contexts, i.e. a context from a different syntax file, things get different.

If you are including a context, it is usually because you want certain patterns. At this point, things get brittle. The external syntax could be modified to rename contexts, or add/remove the prototype, etc. However, if you are specifying a context name, you are getting specific enough that you probably don't want implicitly to get the external prototype. And if you do want the external prototype, you can just include that also.

Now, we could (theoretically) add syntax so when including you could explicitly ask for the prototype to be added:

 - include: scope:source.foo
   include_prototype: true

But that can be accomplished via:

 - include: scope:source.foo#prototype
 - include: scope:source.foo

So we haven't really gained anything. I don't think we'd want to change include to implicitly add the prototype to all external contexts, since then it couldn't be disabled.

I suppose the only benefit that could be had by adding something like include_prototype: would be for it to be "optional" in the sense that if the prototype didn't exist, it would not be an error. Right now if you tried to explicitly include the prototype, and it doesn't exist, it would be an error. However, generally external context includes are pretty brittle anyway, and the two syntaxes need to stay in sync.

Perhaps the simplest option here is to just clearly document how includes work for an external syntax.


The embed action is different than include. I haven't looked at that @rwols, but I would expect it to have different semantics.

@deathaxe
Copy link
Collaborator

What I mainly have in mind when thinking about include is SublimeText/AFileIcon#21. It made me think, we have a general issue with prototype not being applied when including/embedding a syntax.

With your explanations in mind I did some tests on my end using the examples of one of my former comments. I modified them to include the source.yaml syntax which was subject of SublimeText/AFileIcon#21.

The results make me fully agree with your statements above. Sublime Text behaves correctly.

If we directly push/set/embed a scope:source.foo, prototype is applied to it as expected. With the example of YAML we clearly see the comments to be highlighted correctly.

%YAML 1.2
--- 
name: My Syntax
scope: source.my-syntax

contexts:
  main:

    # `source.yaml#prototype` is applied implicitly
    # => comments show correctly
    - match: \`{3}
      scope: punctuation.section.begin
      push: scope:source.yaml
      with_prototype:
        - match: \`{3}
          scope: punctuation.section.begin
          pop: true

    # `source.yaml#prototype` is applied implicitly
    # => comments show correctly
    - match: \`{3}
      scope: punctuation.section.begin
      embed: scope:source.yaml
      escape: \`{3}
      escape_captures:
        0: punctuation.section.begin

grafik

If we include scope:source.foo in an intermediate embedded-yaml context, it is not implicitly applied. Thus YAML comments are not highlighted.

%YAML 1.2
--- 
name: My Syntax
scope: source.my-syntax

contexts:
  main:

    # `source.yaml#prototype` is not applied implicitly
    # => comments not highlighted
    - match: \`{3}
      scope: punctuation.section.begin
      push: embedded-yaml
      with_prototype:
        - match: \`{3}
          scope: punctuation.section.begin
          pop: true

    # `source.yaml#prototype` is not applied implicitly
    # => comments not highlighted
    - match: \`{3}
      scope: punctuation.section.begin
      embed: embedded-yaml
      escape: \`{3}
      escape_captures:
        0: punctuation.section.begin

  embedded-yaml:
    - meta_content_scope: source.yaml.embedded.my-syntax
    - include: scope:source.yaml

grafik

As we have full control about whether to include the prototype, the only issue remaining is indeed a missing prototype context which is tried to be included via - include: scope:source.foo#prototype to throw an error.

If we want to use such intermediate contexts such as embedded-yaml to prevent a syntaxes main scope to be applied (see #2482), we might need some kind of mechanism to optionally include a prototype if present but fail gracefully if it is not.

deathaxe added a commit to deathaxe/sublime-packages that referenced this issue Jun 28, 2020
This commit introduces an intermediate `embedded-javascript` context to
avoid applying the `source.js` scope of the embedded syntax as we want
to scope the section `source.js.embedded.html` instead.

Note: When using `- include` the prototype context of the included
      syntax needs to be included explicitly.
      see: sublimehq/sublime_text#1062)
deathaxe added a commit to deathaxe/sublime-packages that referenced this issue Jun 28, 2020
This commit introduces an intermediate `embedded-css` context to
avoid applying the `source.css` scope of the embedded syntax as we want
to scope the section `source.css.embedded.html` instead.

Note: When using `- include` the prototype context of the included
      syntax needs to be included explicitly.
      see: sublimehq/sublime_text#1062)
@deathaxe
Copy link
Collaborator

If we want keep going with the current solution of an intermediate contexts to avoid a syntaxes main scope from being applied (see #2482), we definitly need something to optionally include a prototype without throwing errors.

The embedded syntax may change, thus the including syntax can't know whether prototype exists or not.

See how sublimehq/Packages#2397 needs to handle CSS and JavaScript differently to satisfy the CI syntax test engine.

@wbond
Copy link
Member

wbond commented Jul 10, 2020

Build 4075 adds the option apply_prototype: true to include: rules. If the referenced syntax has a prototype, and the prototype is not excluded from the context, then the include: rule with include the prototype first, followed by the body of the context.

It also respects the custom meta_prototype rule that I believe hitherto has been undocumented. meta_prototype allows you to specify a context with a different name than prototype to be the context's prototype. In practice I can't imagine it is super useful, since it doesn't save anything over using - include: custom_proto.

@wbond wbond closed this as completed Jul 10, 2020
@wbond wbond added this to the Build 4075 milestone Jul 10, 2020
@wbond wbond added the R: fixed label Jul 10, 2020
@FichteFoll
Copy link
Collaborator Author

FichteFoll commented Jul 10, 2020

Thanks for the fix.

In practice I can't imagine it is super useful, since it doesn't save anything over using - include: custom_proto.

Agreed, it seems quite superficial and exactly equivalent to:

- meta_include_prototype: false
- include: custom_proto

Considering it's not been mentioned anywhere before and probably entirely unused in the wild, I'm not even going to add it to PackageDev.

@deathaxe
Copy link
Collaborator

I would the context specified by meta_prototype to be used in all subsequent contexts instead of it being a plain include.

@wbond
Copy link
Member

wbond commented Jul 10, 2020

What is a "subsequent context"?

@FichteFoll
Copy link
Collaborator Author

FichteFoll commented Jul 10, 2020

Any contexts pushed onto the current context.

@wbond
Copy link
Member

wbond commented Jul 10, 2020

So it would be similar to with_prototype making a copy of any context that is pushed and altering the prototype?

Either way, that isn't how it has been implemented (for a very long time).

@deathaxe
Copy link
Collaborator

Something like this. Would propably just be an alternative syntax for the with_prototype though.

contexts:

  main:
    - include: context1
    - include: context2

  prototype:
    - match: something
      scope: whatever

  my_custom_prototype_of_subcontext1:
    - match: something-else
      scope: whatever-you-want


  context1:
    - match: foo
      scope: foo
      push: sub-context1

  sub-context1:
    - meta_prototype: my_custom_prototype_of_subcontext1
    - match: bar
      scope: baz
      push:
        # includes my_custom_prototype_of_subcontext1
        - match: baz
          scope: haha
          push: 
            # includes my_custom_prototype_of_subcontext1
            - match: haha
              scope: hmmm

  context2:
    - match: bar
      scope: baz
      push: sub-ocntext2

  sub-context2:
    # includes prototype
    - match: bar
      scope: baz
      push:
        # includes prototype
        - match: baz
          scope: haha
          push: 
            # includes prototype
            - match: haha
              scope: hmmm

Note: This is not a feature request, but just the idea of how I would have expected it to work. Don't see a practical use for it at this point.

@deathaxe
Copy link
Collaborator

Imagine this change then.


contexts:

  main:
    - include: context1
    - include: context2

  prototype:
    - match: something
      scope: whatever

  my_custom_prototype_of_subcontext1:
    - match: something-else
      scope: whatever-you-want


  context1:
    - match: foo
      scope: foo
      push: sub-context1

  sub-context1:
    - meta_prototype: my_custom_prototype_of_subcontext1
    - include: sub-context2
    # - match: bar
    #   scope: baz
    #   push:
    #     # includes my_custom_prototype_of_subcontext1
    #     - match: baz
    #       scope: haha
    #       push: 
    #         # includes my_custom_prototype_of_subcontext1
    #         - match: haha
    #           scope: hmmm

  context2:
    - match: bar
      scope: baz
      push: sub-context2

  sub-context2:
    # includes prototype
    - match: bar
      scope: baz
      push:
        # includes prototype
        - match: baz
          scope: haha
          push: 
            # includes prototype
            - match: haha
              scope: hmmm


wbond pushed a commit to sublimehq/Packages that referenced this issue Jul 16, 2020
* [Makefile] Fix embedded ShellScript prototypes

Fixes #2318

Problem Description

  The Makefile.sublime-syntax includes the Bash.sublime-syntax.

  The `scope:source.shell#prototype` is injected into the embedded
  Bash.sublime-syntax using `with_prototype` maybe as workaround for
  sublimehq/sublime_text#1062

  The `with_prototype` command ignores `meta_include_prototype: false`,
  which is heavily used by Bash.sublime-syntax to prevent the
  `prototype` context from being included into a bunch of contexts such
  as strings.

Fix Description

  As the prototype context is not included into source.shell's main
  context only, it is not necessary to inject it via `with_protoype`. Hence it is moved to the `shell-body` together with `source.shell`,
  which is suggested by the core issue as proper workaround.

Notes:

  The `prototype` context of the Bash.sublime-syntax also includes
  proper rules to pop off all contexts at eol with respect of line
  continuation. Hence those don't need to be part of the `with_proptype`
  statement as well. They are moved to just pop off the Bash's main
  context as soon as the eol is reached.

  The `source.shell.embedded` scope is applied to all embedded shell
  syntax regions.

* [Makefile] Fix embedded comment scope

Fixes an oversight with regards of the changed `source.shell.embedded`
scope, which is used for embedded ShellScript syntax.

* [Makefile] Use ST4077 syntax features

This commit uses `apply_prototype` to optionally include the prototype
if present but keep calm if it was not defined by the included syntax.

Add some tests for embedded shell scopes as well.
wbond pushed a commit to sublimehq/Packages that referenced this issue Jul 20, 2020
* [HTML] Fix embedded JavaScript main scope

This commit introduces an intermediate `embedded-javascript` context to
avoid applying the `source.js` scope of the embedded syntax as we want
to scope the section `source.js.embedded.html` instead.

Note: When using `- include` the prototype context of the included
      syntax needs to be included explicitly.
      see: sublimehq/sublime_text#1062)

* [HTML] Fix embedded JavaScript event attribute meta scope

This commit removes duplicated `meta.attribute-with-value.event` scopes
from `event` tag attributes.

The opening single quote was even scoped 3 times.

* [HTML] Add embedded JavaScript event attribute interpolation

This commit applies `meta.string.html meta.interpolation.html` to the
`event` tag attribute value.

* [HTML] Add embedded CSS style attribute interpolation

This commit applies `meta.string.html meta.interpolation.html` to the
`style` tag attribute value.

* [HTML] Fix embedded CSS main scope

This commit introduces an intermediate `embedded-css` context to
avoid applying the `source.css` scope of the embedded syntax as we want
to scope the section `source.css.embedded.html` instead.

Note: When using `- include` the prototype context of the included
      syntax needs to be included explicitly.
      see: sublimehq/sublime_text#1062)

* [HTML] Add tag attribute interpolation

This commit adds `meta.string.html` to the remaining tag attribute
values for consistency reasons with the former commits which address
string interpolation.

It addresses:
- class tag attribute
- generic tag attributes
- id tag attribute

* [HTML] Prepare for merge

This commit replaces the quotes in order to reduce the number and
complexity of merge conflicts with the assumption of quotes-vs-escapes
being merged before.

* [HTML] Remove source.css#prototype

Avoid error messages as CSS doesn't provide a prototype context.

* [HTML] Convert to sublime-syntax version 2

To avoid possible issues with none-existing prototype contexts in 
embedded syntaxes update to sublime-syntax version 2 

a) with a minimal set of required changes to fix some syntax test
   failures and
b) use the normal `embed: scope:...` again, which now omits the main
   scope of the embedded syntax if `embed_scope` is provided.

* [HTML] Cleanup Syntax Header

`scope` and `version` feel better placed right next to the `name` while
file_extensions and first_line_match belong together as separate fields.
@wbond wbond removed their assignment Aug 7, 2020
mitranim pushed a commit to mitranim/Packages that referenced this issue Mar 25, 2022
* [Makefile] Fix embedded ShellScript prototypes

Fixes sublimehq#2318

Problem Description

  The Makefile.sublime-syntax includes the Bash.sublime-syntax.

  The `scope:source.shell#prototype` is injected into the embedded
  Bash.sublime-syntax using `with_prototype` maybe as workaround for
  sublimehq/sublime_text#1062

  The `with_prototype` command ignores `meta_include_prototype: false`,
  which is heavily used by Bash.sublime-syntax to prevent the
  `prototype` context from being included into a bunch of contexts such
  as strings.

Fix Description

  As the prototype context is not included into source.shell's main
  context only, it is not necessary to inject it via `with_protoype`. Hence it is moved to the `shell-body` together with `source.shell`,
  which is suggested by the core issue as proper workaround.

Notes:

  The `prototype` context of the Bash.sublime-syntax also includes
  proper rules to pop off all contexts at eol with respect of line
  continuation. Hence those don't need to be part of the `with_proptype`
  statement as well. They are moved to just pop off the Bash's main
  context as soon as the eol is reached.

  The `source.shell.embedded` scope is applied to all embedded shell
  syntax regions.

* [Makefile] Fix embedded comment scope

Fixes an oversight with regards of the changed `source.shell.embedded`
scope, which is used for embedded ShellScript syntax.

* [Makefile] Use ST4077 syntax features

This commit uses `apply_prototype` to optionally include the prototype
if present but keep calm if it was not defined by the included syntax.

Add some tests for embedded shell scopes as well.
mitranim pushed a commit to mitranim/Packages that referenced this issue Mar 25, 2022
…limehq#2397)

* [HTML] Fix embedded JavaScript main scope

This commit introduces an intermediate `embedded-javascript` context to
avoid applying the `source.js` scope of the embedded syntax as we want
to scope the section `source.js.embedded.html` instead.

Note: When using `- include` the prototype context of the included
      syntax needs to be included explicitly.
      see: sublimehq/sublime_text#1062)

* [HTML] Fix embedded JavaScript event attribute meta scope

This commit removes duplicated `meta.attribute-with-value.event` scopes
from `event` tag attributes.

The opening single quote was even scoped 3 times.

* [HTML] Add embedded JavaScript event attribute interpolation

This commit applies `meta.string.html meta.interpolation.html` to the
`event` tag attribute value.

* [HTML] Add embedded CSS style attribute interpolation

This commit applies `meta.string.html meta.interpolation.html` to the
`style` tag attribute value.

* [HTML] Fix embedded CSS main scope

This commit introduces an intermediate `embedded-css` context to
avoid applying the `source.css` scope of the embedded syntax as we want
to scope the section `source.css.embedded.html` instead.

Note: When using `- include` the prototype context of the included
      syntax needs to be included explicitly.
      see: sublimehq/sublime_text#1062)

* [HTML] Add tag attribute interpolation

This commit adds `meta.string.html` to the remaining tag attribute
values for consistency reasons with the former commits which address
string interpolation.

It addresses:
- class tag attribute
- generic tag attributes
- id tag attribute

* [HTML] Prepare for merge

This commit replaces the quotes in order to reduce the number and
complexity of merge conflicts with the assumption of quotes-vs-escapes
being merged before.

* [HTML] Remove source.css#prototype

Avoid error messages as CSS doesn't provide a prototype context.

* [HTML] Convert to sublime-syntax version 2

To avoid possible issues with none-existing prototype contexts in 
embedded syntaxes update to sublime-syntax version 2 

a) with a minimal set of required changes to fix some syntax test
   failures and
b) use the normal `embed: scope:...` again, which now omits the main
   scope of the embedded syntax if `embed_scope` is provided.

* [HTML] Cleanup Syntax Header

`scope` and `version` feel better placed right next to the `name` while
file_extensions and first_line_match belong together as separate fields.
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