Proposal for CSS3 to replace the default CSS package

[y0ssar1an]

I’ve just opened an issue on sublimehq/Packages for CSS3 to replace the default CSS package. My reasoning boils down to this:

• There are way too many bugs in the default CSS package to ever fix them all.
• Improving the existing CSS package would be a major effort. I don’t think it’s worth the core Sublime dev’s time to spend on just one language.
• The work has already been done.

CSS3 isn’t free of highlighting mistakes, but it has a lot fewer of them. It’s basically a comprehensive implementation of the 40+ W3C specs that define CSS. It’s a Top 100 package on Package Control, so it’s pretty battle-hardened.

I think it would be a big improvement for Sublime’s out-of-the-box experience, but I’d like to hear what the community thinks. If my proposal is considered, I would really appreciate code review from all you plugin experts . Thanks in advance for the feedback!

[wbond]

We’ve significantly revised the CSS package over the past year, and added a ton of syntax tests. If you know of any bugs, please report them.

We’ve done a wholesale replacement of a default package once, with JavaScript, and it did not go well. All of the other significant changes to packages have taken a much more evolutionary approach, and seem to have gone well. Because of this, I am not inclined to replace CSS, but instead identify and fix any bugs.

We’ve got a small, but dedicated group of contributors who have been helping to push all of the default syntaxes forward. Between those contributions and my own work on the syntaxes, I think we should be able to resolve any issues with the CSS syntax.

[addons_zz]

Outside looking I think it is sad to have 2 different developers teams developing the same thing. It would be like constantly wring the same thing twice. Seems not efficient/helpful.

Over looking on the CSS3 package seems to have a lots of work employed on it. Though, may be this work could be integrated with the default CSS package, so we would not need choose between one or the other, as both works would be integrated, as they become the same package.

The same would apply to their development teams being united. Therefore we would not have more teams writing the same thing twice. Unless they are not developping the same thing, but as there is proposal for replacing one with the other, they should be pretty related.

[y0ssar1an]

Thanks for the quick reply, @wbond!

I definitely have noticed the improvement in the default CSS package. Kudos to all the contributors! I think the default package does an okay job of covering the most commonly used CSS code. However, the number of issues I’d have to open to get the default package to where CSS3 is today… it would be in the hundreds at least. The full CSS language is just way bigger than the subset supported by the default CSS package. The CSS3 syntax definition is 10,279 lines long, vs. 2002 lines in the default package. A lot of that is the design choice to write a rule for every property, but a lot of that is properties, @-rules, functions, values, selectors, pseudo-classes, pseudo-elements, etc. that are missing or incomplete in the default package.

With that said, this would definitely be a big change. You understand the risks much better than I do. Are you concerned about breaking 3rd party packages that depend on the scopes from the default package? Would it be possible to scan the Package Control corpus to look for packages that might break? Are there other concerns I’m missing?

[wbond]

Scope naming that follows the guidelines, and is generally consistent with historical scope names for CSS are important. Not utilizing regex patterns that trigger Oniguruma is important. Passing the existing test suite is very important. If there are examples of ways it is broken, or scopes that are completely contrary to other syntaxes, we want to address those. However, wholesale replacement tends to throw the baby out with the bathwater. I don’t want to regress on all of the bug fixes we’ve made, or change the scope names we’ve decided are the right thing. I think if a PR doesn’t pass the existing test suite, and can’t clearly list why changes need to be made to the tests, it definitely soaks up a ton more time, and to questionable benefit.

Honestly, if it isn’t possible to enumerate what is wrong with the CSS syntax, that makes me think that the CSS3 syntax is too different to use it as a basis for the CSS package. Probably the biggest benefit would be tests (or even examples) of CSS syntax where the existing syntax fails. If such tests exist, they can be ported over, however scope names often have to change. The fact that the syntax definition is 5 times as long makes me less inclined to try and reconcile the two.

In general I think there is space for default package for a language and a third-party package also. Some package authors like to get hyper specific with scope names, or don’t like the scope naming guidelines, or want to add features that we aren’t ready to commit to supporting as a baseline for the default packages. Packages such as ECMAScript and MagicPython come to mind.

The current approach we have to default syntaxes is to provide the fastest, and most semantically correct highlighting we can. Beyond that we want to ensure that syntaxes share a common set of scope names so that color schemes authors don’t need to try and target specific syntaxes, and we want the syntaxes to be well tested, with regression tests for bugs that have been fixed. With all of these we also need to make sure the syntaxes can be reasonably maintained and don’t soak up too much time relative to the core application.

In the end, it is kind of a big balancing act. So far, I’ve heard your concern that the CSS package is broken in many ways, but I haven’t heard anything yet that makes me think adopting CSS3 is the right solution. So far, it just sounds like a ton of work to try and reconcile the two. Considering the absolutely terrible shape some of the other default syntaxes are in, I’d probably err on the side of spending time getting them to a non-terrible state, and letting advanced CSS users install CSS3.

As always, I’m willing to be convinced otherwise, that is just where I stand right now.

[y0ssar1an]

A couple more points:

1. I tried running the CSS syntax tests with CSS3. 1734 of 2512 assertions failed. The scope names are close in many of the failures, but yikes.
2. CSS3 doesn’t trigger Oniguruma.
3. I threw together a sample file full of places where CSS breaks. This is a worst-case scenario, but it should help clarify how much work needs to be done on the default package. There are many more bugs besides these.

https://gist.github.com/y0ssar1an/ef71dfe49ada9405276d37cb670dab84

I think there are probably too many scope name differences to cleanly swap in CSS3, but in any case, thanks very much for the feedback!

[rwols]

3. I threw together a sample file full of places where CSS breaks. This is a worst-case scenario, but it should help clarify how much work needs to be done on the default package. There are many more bugs besides these.

I think most of these problems are related to each other and a fix for one would fix a whole bunch of similar ones.

Also, scoping tokens as deprecated is risky, because not everyone can use CSS3, e.g. due to business policies or whatever.

[braver]

Also, scoping tokens as deprecated is risky, because not everyone can use CSS3

Additionally, a bunch of stuff in that file is in CSS4, which is still in Working Draft. I mean, I like the CSS3 package, but it isn’t for everyone.

[Stash]

Also, scoping tokens as deprecated is risky, because not everyone can use CSS3

Additionally, a bunch of stuff in that file is in CSS4, which is still in Working Draft. I mean, I like the CSS3 package, but it isn’t for everyone.

I agree, but this shouldn’t take away from the fact that CSS3 does seem to do a better job at highlighting modern CSS code.

Perhaps things like visual indicators for deprecated tags and CSS4 could be an optional switch somewhere?

[ThomSmith]

Perhaps things like visual indicators for deprecated tags and CSS4 could be an optional switch somewhere?

At this time, it isn’t possible to include optional switches in syntaxes. I’m hoping next week to update YAML Macros to support this, but it is unlikely that this functionality would be integrated into a core package.

[deathaxe]

I generally agree with @wbond’s opinion about default packages, but a first short look into the default CSS syntax reveals many regex rules I’d never would think about using. Many unused captures and strange whitespace queries or empty lookaheads like (?=.*?).

[wbond]

a first short look into the default CSS syntax reveals many regex rules I’d never would think about using.

I know that some of those exist, and are necessary, to have scoping work correctly so that completions work as expected. It may be worth a shot changing some of those and seeing if any tests fail.

[addons_zz]

empty lookaheads like (?=.*?).

Look ahead apply scopes by push with meta_scope. For example:

  at-keyframes:
    - match: (?=\s*@(?:-webkit-|-moz-|-o-)?keyframes\s*.*?)
      push:
        - include: rule-list-terminator
        - match: \s*((@)(-webkit-|-moz-|-o-)?keyframes)(?=.*?)
          captures:
            1: keyword.control.at-rule.keyframe.css
            2: punctuation.definition.keyword.css
            3: support.type.property-vendor.css
            4: support.constant.keyframe.css
          push:
            - meta_scope: meta.at-rule.keyframe.css
            - match: '\s*(?=\{)'
              pop: true
            - match: '\s*(?=[^{;])'

The lookahead (?=.*?) will apply the scope meta.at-rule.keyframe.css to its matching characters. As discussed on:

1. https://github.com/SublimeTextIssues/Core/issues/1796#issuecomment-312920245

%YAML 1.2
---
name: test
scope: source.syntax_name

contexts:
  main:
    - match: '(?=looking)'
      push:
        - meta_scope: first.looking.assignment
        - match: '(?=looking)'
          push:
            - meta_scope: second.looking.assignment
            - match: '(?=looking)'
              push:
                - meta_scope: third.looking.assignment
                - match: '(?=looking)'
                  push:
                    - match: 'looking'
                      scope: finally.i.am.capturing.it
                      pop: true
                - match: ''
                  pop: true
            - match: ''
              pop: true
        - match: ''
          pop: true

Currently lookahead are ignored by the captures on capturing groups inside lookaheads, but meta_scope apply the scopes if the characters inside the lookahead are eaten later inside after the meta_scope on push.

[deathaxe]

I know what they do but still wondering why they are used as they are, because I never needed to do something strange like that to make scoping and meta scoping work properly. These constructs even require the lexer to match the pattern twice which I expect to slow down the whole lexing process. What I mean, there are cleaner solutions to achieve the same goals.

[deathaxe]

Already did on some places without negative results. But I can’t evaluate how complete the tests are as I am not that familiar with CSS. Completions should not break if meta and keyword scopes keep accurate, I think.