[ASP] remember ASP context block scope after embedded HTML

[keith-hall]

As per the discussion here, this PR is to ensure that ASP correctly remembers what scope it should be in after embedded HTML.

Performance
Using the syntax test file from this PR, which is 32 KiB:

• ASP syntax definitions before the changes in this PR:

  Syntax "Packages/ASP/HTML-ASP.sublime-syntax" took an average of 2.1ms over 10 runs

• ASP syntax definitions after the changes in this PR:

  Syntax "Packages/ASP/HTML-ASP.sublime-syntax" took an average of 3.2ms over 10 runs

[keith-hall]

currently the scope inside the ASP If block in the JavaScript is text.html.asp text.html.basic source.js.embedded.html source.asp.embedded.html meta.if.block.asp text.html.basic, which means it is not highlighted as JS, but as HTML currently. I would welcome suggestions on DRY ways of fixing this - I had the idea of changing the html context inside HTML-ASP.sublime-syntax to include a match for <script in the prototype, but then I think it would be necessary to duplicate all the contexts in ASP.sublime-syntax to push the JavaScript scope instead of HTML - and we would have the same problem about remembering the JS block scope here too...

In my opinion, it is bad coding practice to mix server side and client side code in this way, but I can see that it would have its uses...

[michaelblyons]

If I'm understanding this correctly, you've added the closing transitionary punctuation (%>) to the ASP.sublime-syntax file to make this work. Would it be kosher for me to make similar changes to C#.sublime-syntax if I were making an HTML-C#.sublime-syntax?

I expected to make HTML-C# its own package, since it's not already part of the core set, but then to inject structures into the core C# syntax that don't do anything but support my third-party syntax feels a little dirty.

[keith-hall]

If I'm understanding this correctly, you've added the closing transitionary punctuation (%>) to the ASP.sublime-syntax file to make this work.

that's correct - it's the easiest way I could think of doing it. I guess an alternative to making any changes to the main language definition file would be to have a prototype in the supporting HTML syntax definition that will look ahead for where the control flow blocks start (and end) and hmm..haven't really thought this through - it would need to push a new context onto the stack that would somehow magically include the current context (or maybe just the statements context or equivalent?) and also look for the transitionary punctuation. Not sure if that makes sense?

[michaelblyons]

Rather than add extra rules to the C# syntax, I modeled the <% ... %> blocks after HTML's injection of Javascript:

  block_cs:
    - match: '<%[=#:]?'
      scope: punctuation.section.embedded.begin.cshtml
      push:
        - meta_scope: source.cs.embedded.html
        - match: '%>'
          scope: punctuation.section.embedded.end.cshtml
          pop: true
        - match: ''
          scope: punctuation.definition.tag.end.html
          push:
            - meta_content_scope: source.cs.embedded.html
            - include: 'scope:source.cs'
          with_prototype:
             - match: '(?=%>)'
               pop: true

This seems to work with <% if(foo){ %> some html <% } %>, and surprisingly (to me) the braces are accurately matched together and underlined when I click on one.

---

Update: Now publicly viewable with a couple changes since the excerpt above. I notice that the closing brace in the test file (<% } %>) does not have a punctuation.section.block.end.source.cs, but the brace matching still finds it.

[wbond]

From experience, adding a pop to the main context can lead to funny interactions when embedding the syntax in another. It may make more sense to have the HTML ASP use a construct such as:

  - match: '<%'
    push:
      - match: '(?=%>)'
        pop: true
      - include: 'scope:source.asp'

[keith-hall]

Thanks, I have added a new commit to implement this suggestion :)

[wbond]

@keith-hall It seems like you've been following work on the various syntaxes pretty closely. Based on that, are you happy with this current implementation? The number of test assertions you have seems to be pretty good. Do you think this PR needs a light review, or something a little more in depth?

[keith-hall]

@wbond I plan to go over this with a fine toothcomb myself first, because I think I learned a few things about scope conventions since submitting this PR. After which, I believe a light review will be sufficient :) I will let you know when it's ready :)

[keith-hall]

@wbond ready for review :) I am happy with this implementation, but I plan to study the changes you recently made to the PHP syntax, just in case we will later decide to mirror those changes in the ASP syntax definition.

If you want me to squash this before you merge, let me know - I had quite a few scope names to correct ;)

[keith-hall]

here is one scope that I am not sure about. In ASP, to set a variable to an object, as opposed to a number or string, it is necessary to use set var_name = obj_reference instead of just var_name =...
also, Call is completely optional, but if used, must be followed by a method name (i.e. a callable object)
so far I have kept the scope as it was previously, but I believe that a keyword scope would be more appropriate. As far as I know, these are unique to VB and ASP. Thoughts?

[FichteFoll]

keyword sounds more appropriate to me too.

[keith-hall]

updated. I've made the scopes unique, and in the same style as some PHP keywords, so that users can modify their color scheme if they don't like the new coloring.

[wbond]

@keith-hall The new clear_scopes feature will probably be useful to you to implement reasonable HTML embedding. It will be part of build 3115.

In summary, it is applied like meta_scope (to push and pop tokens, and all contents of a context), however it erases scope names from the stack out from under the context it is added to. The true value erases all scope names, or you can specify a number of scope names via an integer.

The PHP pull request uses it to erase all of the scopes when literal HTML is encountered, and then uses meta_scope to add back the "base" scopes. This allows it to work with any level of nesting of curly braces, and allow scope selectors to stay that same, so snippets and completions don't need to be updated.

I'll look through this PR soon, but I figured I'd mention that now. Perhaps we wait until 3115 is released so you can test against that, or perhaps I'll pull in your changes into another PR and test it locally.