search

SublimeText / Sass

Sass and SCSS syntax for Sublime Text
https://packagecontrol.io/packages/Sass
MIT License
50 stars 8 forks source link

changed captures for @include, =, +; also fixes issue #84 #87

Closed thie1210 closed 2 years ago

braver commented 2 years ago

I appreciate the pull request, but it breaks the tests and I don't see why your change here is correct or better. I'm also not sure what problem it tries to solve (I don't use Emmet)?

thie1210 commented 2 years ago

Actually, after further investigation, Emmet has nothing to do with it.

Here’s a screenshot with your latest version (d3d94046409db6fbbc9d51dea52b589ecc9d3d48). The scope variable.function.sass is detected on the @include keyword. It seems to me that it should only be detected on the fresh variable. That’s fixed with Sass.sublime-synthax:807-813.

sublime-sass-head

The fix also allows sublime to properly detect the definitions and references with the cursor over the fresh variable—see screenshot below.

sublime-sass-definitions-references

Now for Sass’s “Indented Mixin Syntax,” the fix provides correct highlighting as well as detection of definitions and references—see screenshot below.

sublime-sass-indented-synthax

Please let me know what’s broken (which lines I guess) in the test because on my side they seem to be fine.

braver commented 2 years ago

Ah, yes, you're totally correct. There are some syntax tests you can run inside Sublime Text that help guard against regression. This is what's up in the Sass syntax:

Screenshot 2021-09-21 at 21 12 51
thie1210 commented 2 years ago

Fantastic!

braver commented 2 years ago

A 3.0 beta release is out now, check the readme on how to get the early builds to take advantage of the improvements and help out improving the package before we ship it to the world. Currently I won't handle pull requests into master until that release has been merged. And when it does your PR here will probably conflict. So maybe you can have a look at that beta version (based on #86 ) and if needed propose your improvements for that?

braver commented 2 years ago

This should be solved in 3.0, or otherwise needs to be re-investigated. Let me know if we should re-open to fix this in 3.x.