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

[ST4] A whole bunch of fixes and improvements. #216

Merged
merged 22 commits into from Feb 1, 2023
Merged

[ST4] A whole bunch of fixes and improvements. #216

merged 22 commits into from Feb 1, 2023

Conversation

deathaxe
Copy link

@deathaxe deathaxe commented Sep 9, 2021

This PR applies further fixes and improvements with regards to correct meta scopes, close-tag handling and aligns the overall syntax structure with ST's core HTML.sublime-syntax.

For details about the single changes, please refer to commit messages.

Improve readability.
This commit...

1. fixes duplicated meta.tag scopes in `<template>` tags
2. fixes missing meta.tag scope at the closing `>` in `<template>` tags
3. adds syntax tests for template tags
This commit fixes implementation of mustage template tags {{...}}.

Before this commit they tried to pop the `main` context off stack
and caused `template-tag-content` to never have been popped off stack
properly.

With this commit the syntax distinguishes between embedded template
tags and those which interpolate strings. The latter one remove
`string` scope for better highlighting java code in `{{ ... }}`.

It also does no longer push a dedicated `mustage-content` context on
stack as this is the same content as the whole document. It's not
required.
All `v-foo` directive attributes have not been scoped
`meta-attribute-with-value` which effects syntax highlighting when
using certain color schemes. That's fixed by this commit.

Note: It appears YAMLmacros sorts generated output randomly, so many
changes of the generated sublime-syntax file can be ignored.
This commit refactors close-tag context handling in order to fix meta
scopes of stray closing tags.

Before this commit they received the default meta and entity scope from
HTML.sublime-syntax. This may result in inconsistent highlighting once
they are directly addressed by a color scheme.

That's a fix which is planned for core HTML.sublime-syntax, too.
The change of `script-tag` and `style-tag` also fixes duplicated
meta.tag scopes for script/style tags for ST builds before 4114.
This commit just moves sections/contexts around. It doesn't change
behavior.

The goal is to end up with the same order as HTML.sublime-syntax which
uses a top-down-order.

1. managing contexts
2. tag contexts
3. attribute contexts
4. other reusable contexts
5. prototypes
All sublime-syntax version 2 files should use explicit `pop: 1` instead
of `pop: true`. It looks more consistent in case `pop: 2` are used.
No need to duplicate the match directive if it already exists.
This commit fixes sublimehq/Packages#2910 for
all ST builds before 4114.
@rchl
Copy link

rchl commented Sep 9, 2021

Working well for me. Not noticing any differences on the surface.

ST's HTML.sublime-syntax may change in the future so that the change of
this commit is required to maintain compatibility. It is planned to
highlight certain self-closing tags illegal, if they are known not to
support it.

This change is backward compatible with older builds of ST4.
@deathaxe
Copy link
Author

deathaxe commented Feb 2, 2022

This PR ensures correct meta.scopes, cleans some things up and ensures compatibility with a possible future change to ST's core HTML syntax.

Addresses #216 (comment)

This commit includes `JavaScript#expression-statement` context in
attribute/directive values to highlight mappings correctly.

The `pop: 1` is removed as the included context pops itself off stack.
Run YAML macros in python 3.8 in order to maintain dictionary key sort
order.
@FichteFoll
Copy link

FWIW, the changes here revert #168. I now switched to use source.js - meta.attribute-with-value as a selector for linting, but not sure if this is what we'd want here. It's better than pre-#168, though.

@deathaxe
Copy link
Author

deathaxe commented Mar 4, 2022

Means, you'd suggest to use .embedded.vue instead of .embedded.html for all embedded languages?

@FichteFoll
Copy link

Not precisely, no. I don't have a clear picture on how embedded languages are scoped currently, but I suppose it does make sense to differentiate between different embedded languages by denoting the text/source scope with the language whose syntax is responsible for the embedding if we're going to include some identifier there. Thus, since the attribute syntax is most definitely vue-specific, that is a candidate for .embedded.vue over .embedded.html, whereas I don't think so about the <script> block, since that uses standard HTML syntax.

Regardless, for the mentioned purpose, the meta scope for HTML attributes is certainly enough.

This allows easily targeting the embed scopes inside attributes as
something that is contributed and supported by the vue syntax only as
opposed to a a `<script>` tag which uses HTML syntax.
@FichteFoll
Copy link

I have made further additions to this PR in my fork and filed a PR against this branch in https://github.com/deathaxe/sublime-vue/pull/1.

FichteFoll and others added 4 commits January 5, 2023 17:19
Use vue suffix for embed_scope in attributes and mustache
This commit moves terminating patterns to the top so they match with
max priority in case of overlapping patterns. This change is just to
be future proof if core HTML syntax changes.
This commit follows a scope naming scheme introduced to Astro, Blade, Liquid, PHP and Slim recently, which uses

1. `meta.embedded.<template-name>` for those template code sections,
   which are used for control structures primarily.

   This scope has already been in use in PHP syntax to scope all
   `<?php ... ?>` code sections.

2. `meta.interpolation.<template-name>` for those template code
    sections, which primarily emit data to the output document.

    That's what Mustage tags `{{ expr }}` do primarily.
@deathaxe
Copy link
Author

deathaxe commented Jan 5, 2023

PR is merged with two further scoping changes, already applied to various other template syntaxes such as Astro, Blade, Liquid and Slim.

1. extend `meta.interpolation` to include leading and trailing
   punctuation to comply with ST's scope naming guidelines.
2. fix closing `]` scope.
3. adjust/add tests
@skyronic
Copy link
Collaborator

skyronic commented Jan 5, 2023

Hello @FichteFoll @deathaxe - thank you for your time and contribution. I'll just look over this in the next couple of days and merge it in if I don't find any issues.

Would you be able to provide a summary of changes/improvements for the changelog?

@deathaxe
Copy link
Author

deathaxe commented Jan 6, 2023

The following list should contain the most significant parts. Not sure if all details need to be mensioned in changelog though.

Changelog

  • adjust syntax code structure to basic HTML.sublime-syntax for better maintainability

    • reorder contexts
    • add some "section headers" aka separators for better code navigation
    • re-use existing contexts such as tag-end-self-closing and tag-end
    • minor code style tweaks (e.g.: pop: 1 or main key order)
  • Fix mustage template interpolation ...

    • possibly popping main context off stack, breaking whole syntax.
    • not being highlighted within attribute values and strings.
    • scope them meta.interpolation instead of meta.template
      to comply with Astro, Blade, Liquid, PHP, Slim, ...
  • Add attribute name interpolation support (e.g.: <p v-#[ name expr ])

  • Fix vue directive attributes not being scoped meta.attribute-with-value

  • Fix stray </style> or </script> tags' meta scopes.
    They are now correctly scoped meta.tag.[style|script].

  • opt-in to python 3.8 plugin_host (used for YAMLMacros)

  • add more syntax tests to ensure not to break syntax by future changes.

@FichteFoll
Copy link

FichteFoll commented Jan 6, 2023

  • Add attribute name interpolation support (e.g.: <p v-#[ name expr ])

To reiterate on that, there are actually two additions here.

  • Add attribute name interpolation support (e.g.: <p :[ name expr ]="value expr">)
  • Add support for slot shorthand syntax (e.g.: <p #default="props">)

@deathaxe
Copy link
Author

deathaxe commented Feb 1, 2023

Merging this PR may become important for end users due to sublimehq/sublime_text#5852

@skyronic
Copy link
Collaborator

skyronic commented Feb 1, 2023

Thank you! I did some testing and everything seems to work good. I'm going to merge this, and will make a release after a bit more testing.

@skyronic skyronic merged commit ec6e5e7 into vuejs:st4 Feb 1, 2023
skyronic pushed a commit that referenced this pull request Feb 1, 2023
Addresses #216 (comment)

This commit includes `JavaScript#expression-statement` context in
attribute/directive values to highlight mappings correctly.

The `pop: 1` is removed as the included context pops itself off stack.
@skyronic
Copy link
Collaborator

skyronic commented Feb 2, 2023

Thanks again for the great work folks, the PR has now been released as part of 4.1.0. I tested it with a wide variety of vue files in different open source projects and everything seemed to work great.

Please let me know if there are any issues with upgrading!

@deathaxe deathaxe deleted the st4 branch February 2, 2023 18:51
@deathaxe deathaxe restored the st4 branch February 2, 2023 18:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants