SublimeText / Sass

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

Match css rewrite #86

Closed braver closed 2 years ago

braver commented 3 years ago

The default CSS package was rewritten to solve various issues and align more closely with the newest scope naming guidelines. The changes are too big to "cherry pick", so we need to start over again as well. This PR tracks the process of starting from a fresh copy of the CSS package and then re-implementing the Sass language features on top of it. In the process we also simplify it around the sass-value-expression.


Fixes #85 #82 #72 #69 #81

jrappen commented 2 years ago

Update the workflow? Compare:

https://github.com/sublimehq/Packages/blob/master/.github/workflows/ci.yml

Just keep in mind the subfolders in the Packages repo vs the root dir here.

braver commented 2 years ago

Oh, that can be updated as well, but for now it is working... there are just quite a few tests that need looking at.

jrappen commented 2 years ago

Fair enough, though as a quick fix you might want to test against 4094 for the time being, as that was the last Build with syntax related changes.

jrappen commented 2 years ago

Was there a reason for not using Inheritance?

braver commented 2 years ago

Was there a reason for not using Inheritance?

From Discord:

There are so many places I need to hook into to add support for expressions it becomes really hard to use inheritance. But more importantly, I cannot follow the Sublime release schedule because I simply don't have time. So whenever ST changes the CSS syntax that would potentially break LESS, SASS and SCSS. ST4 definitely would've broken those syntaxes. Since I can't pin my dependencies on a specific version, I cannot use dependencies.

jrappen commented 2 years ago

@braver The issue with d51e5e0 was: It should be *.tar.xz for the test binary, not *.tar.bz2. It changed around 4078 or so.

I see you're already working on it...

jrappen commented 2 years ago

@braver The Packages workflow expects subfolders. See comment above or compare with the one you replaced.

braver commented 2 years ago

I don't see the difference with what I had before, but now it's not seeing the yaml syntax I depend on. I'm too tired for this....

jrappen commented 2 years ago

I'll take another look maybe later tonight.

braver commented 2 years ago

Thanks, really appreciate it 👍🏻

jrappen commented 2 years ago

@braver I'd assume you'll need to pull in the Packages repo, if you depend on YAML.

jrappen commented 2 years ago

Randy rewrote the UnitTesting plugin. I believe you can use stuff from there.

Or the usual suspect, PackageDev with syntax-test-action.

braver commented 2 years ago

I guess it is seeing and interpreting my syntax correctly. In previous versions I didn’t have to do anything for the yaml stuff, but pulling in that syntax seems reasonable.

Op 30 nov. 2021 om 21:54 heeft Johannes Rappen @.***> het volgende geschreven:

 Randy rewrote the UnitTesting plugin. Have a look there? I believe you can use stuff from there.

Or the usual suspect, PackageDev with syntax-test-action.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe. Triage notifications on the go with GitHub Mobile for iOS or Android.

jrappen commented 2 years ago

Quickest fix should be using syntax-test-action with:

image
braver commented 2 years ago

Exactly, but before this PR is also had exactly that and the workflow did work then. Did previous versions of what it’s using from ST contain packages?

Op 30 nov. 2021 om 22:06 heeft Johannes Rappen @.***> het volgende geschreven:

 @jrappen commented on this pull request.

In Syntaxes/Sass.sublime-syntax:

  • frontmatter:
  • Yekyll front matter

  • match: '^---$' scope: frontmatter.jekyll punctuation.section.frontmatter.begin.jekyll embed: scope:source.yaml (2) and this

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe. Triage notifications on the go with GitHub Mobile for iOS or Android.

jrappen commented 2 years ago

Maybe you're never hitting that unchanged line? Just a random guess. Didn't have time to investigate.

braver commented 2 years ago

Could be. Thanks for all the tips and examples. I’ll figure it out after I get some sleep 😅

Op 30 nov. 2021 om 22:18 heeft Johannes Rappen @.***> het volgende geschreven:

 Maybe you're never hitting that unchanged line? Just a random guess. Didn't have time to investigate.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe. Triage notifications on the go with GitHub Mobile for iOS or Android.

niksy commented 2 years ago

Was there a reason for not using Inheritance?

From Discord:

There are so many places I need to hook into to add support for expressions it becomes really hard to use inheritance. But more importantly, I cannot follow the Sublime release schedule because I simply don't have time. So whenever ST changes the CSS syntax that would potentially break LESS, SASS and SCSS. ST4 definitely would've broken those syntaxes. Since I can't pin my dependencies on a specific version, I cannot use dependencies.

Is this something that can be communicated with Sublime Text team so they can add basic support where we can hook in without rewriting everything? Maybe that’s something which can be useful to other syntaxes.

jrappen commented 2 years ago

FYI for ST4081+ there's also view.syntax() and sublime.Syntax as an alternative to using view.match_selector.

braver commented 2 years ago

Progress: updated the completions file, and improved how property value completions work in SCSS. I notice the meta scopes in Sass could be better though, and I really need some time to look into this.

where we can hook in without rewriting everything

There is nothing they can do to help speed up the development of this syntax. It really doesn't matter, in practice, if I copy-paste some of the CSS syntax, or if I can inherit them. I'm not rewriting their bits and pieces. Sure, it would be cleaner if I could depend on their work, but that's simply in theory impossible to do in a way that ensures they don't break my work.

The effort here in this branch isn't about copy-pasting stuff, it's making a syntax that is structurally different match up with an existing syntax. You'd think the nesting nature of SCSS is a minor thing, but it completely changes the game because anything can be a selector or a property and you can only be sure in retrospect. This is especially challenging in completions. You can't use meta scopes the same way to limit what can be matched, which means various bits I borrowed from the CSS syntax also need to be tweaked.

I the end, it's just a lot of work, and if then the workflow also breaks, not all of it is actually any fun.

braver commented 2 years ago

woop, passing tests now 💪🏻