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

Should we highlight all CSS discrete values? #3615

Closed
joshgoebel opened this issue Sep 11, 2022 · 12 comments
Closed

Should we highlight all CSS discrete values? #3615

joshgoebel opened this issue Sep 11, 2022 · 12 comments
Labels
discuss/propose Proposal for a new feature/direction help welcome Could use help from community language

Comments

@joshgoebel
Copy link
Member

joshgoebel commented Sep 11, 2022

Split from the discussion in #3601:

I'm not even yet convinced that all values should be highlighted. Highlighting a thing well is as much about what one chooses to highlight as what one does not.

In rethinking this just now I'm not sure we should even add colors - unless we first think whether the plan is to include a list of ALL discrete values. I think before we go down that road I'd want to know how large a size change this is... would the CSS grammar going to be 10% larger or 200%?

Hint at valid CSS and valid CSS properties and values

We're not interested in what is "invalid" as this is very hard to do with pattern matching across ALL grammars - and we desire consistency in features we add to the library. If adding invalid to all grammars would be difficult, we're not interested in adding it to only a few (just because they are easy).

Originally posted by @joshgoebel in #3601 (comment)

@joshgoebel
Copy link
Member Author

joshgoebel commented Sep 11, 2022

Things we need to know:

  • The full list of all valid named CSS values
  • What scope should we use for values? literal, built-in, string, etc?

Considerations:

  • Yet another huge list means additional maintenance in the future...

@joshgoebel
Copy link
Member Author

joshgoebel commented Sep 11, 2022

So far we're in good company with GitHub, who also does NOT highlight values: https://github.com/highlightjs/highlight.js/blob/main/demo/style.css

Update: Also Prism.js doesn't highlight CSS values.

@joshgoebel joshgoebel added help welcome Could use help from community discuss/propose Proposal for a new feature/direction labels Sep 11, 2022
@velara3
Copy link

velara3 commented Sep 11, 2022

I think many things are on as needed basis and I think may be the case here.

But there is a distinction, even Github makes.

In this case, as pointed out, CSS attribute values haven't been universally highlighted. Some values are highlighted while some are not.

See exhibit A for values currently highlighted:

  // attribute values
  {
    begin: ':',
    end: '[;}]',
    contains: [
      modes.HEXCOLOR,
      modes.IMPORTANT,
      modes.CSS_NUMBER_MODE,

Case:
If a theme author wants to highlight property names they have the class hljs-attribute. If they want to highlight style values they don't have a class for that but some values do get highlighted (numbers, colors, etc). In this case you get hljs-number.

Things we need to know:

The full list of all valid named CSS values
What scope should we use for values? literal, built-in, string, etc?
Considerations:

Yet another huge list means additional maintenance in the future...

Agreed. In my humble opinion, a list of valid CSS values would be too large and would need patterns to match things like matrix values, rotation, gradients, variables and all that. I don't think we should try and validate any CSS beyond keywords.

For color names, you could create a list. I didn't count but there looks to be about 200 named colors? Reference:

https://www.w3.org/wiki/CSS/Properties/color/keywords

My opinion is, I would simply have a fall back match for any string up to the semicolon. But maybe I'm missing something. What do you mean by "discrete values"?

@joshgoebel
Copy link
Member Author

and would need patterns to match things like matrix values, rotation, gradients, variables and all that.

Those would indeed require separate modes and might be in-scope... we already match some of this with our support for "functions", etc...

I would simply have a fall back match for any string up to the semicolon.

There is no guarantee of a semi-colon. Any changes we make to CSS must also be applicable to the CSS-variants which do not have such syntactic elements. #2269 The simplest way to guarantee consistent behavior across all 4 grammars is with explicit lists of keywords.

What do you mean by "discrete values"?

I mean known/named values: block, left, auto, etc...

@velara3
Copy link

velara3 commented Sep 11, 2022

I'd like to add that not all property names are highlighted. See Exhibit A:

image

#3606

So, if the highlighter is highlighting by known CSS style names it will miss unknown style names (see SVG).

If you or we decide to highlight non number or color values should we also highlight non known property names?

@joshgoebel
Copy link
Member Author

This is on-purpose to support all 4 CSS like grammars - where you cannot use syntactic elements (like :) to "guess" at what a thing is or isn't.

One could for sure write a pure CSS grammar this way (indeed years go that may even have been the case, I forget), but that's not the direction we chose here - because of the desire/need for consistent behavior across all 4 grammars.

@velara3
Copy link

velara3 commented Sep 11, 2022

I would simply have a fall back match for any string up to the semicolon.

There is no guarantee of a semi-colon. Any changes we make to CSS must also be applicable to the CSS-variants which do not have such syntactic elements. #2269 The simplest way to guarantee consistent behavior across all 4 grammars is with explicit lists of keywords.

That case was mentioned in the docs. That you have a begin but not an end.

This is on-purpose to support all 4 CSS like grammars - where you cannot use syntactic elements (like :) to "guess" at what a thing is or isn't.

One could for sure write a pure CSS grammar this way (indeed years go that may even have been the case, I forget), but that's not the direction we chose here - because of the desire/need for consistent behavior across all 4 grammars.

I see. Then I can't comment on the difficulty of that but it seems the attribute value is already being matched.

@joshgoebel
Copy link
Member Author

That case was mentioned in the docs. That you have a begin but not an end.

Not sure I follow?

@velara3
Copy link

velara3 commented Sep 11, 2022

There was a comment in documentation on specifically that case. Where you'd have a style rule where the style name and value would be present but may or may not have an ending semicolon. It sounded like the CSS rule match allowed for that case.

image

https://highlightjs.readthedocs.io/en/latest/mode-reference.html?highlight=semicolon#excludebegin-excludeend

image

https://highlightjs.readthedocs.io/en/latest/mode-reference.html#endswithparent

@joshgoebel
Copy link
Member Author

Then I can't comment on the difficulty of that.

It might be doable, but it's more complex - and we need the keyword lists anyways for auto-detection support. So we need to maintain a pretty large list in any case (as long as we do autodetect). So as there are multiple reasons for the keyword lists, we just use them.

but may or may not have an ending semicolon.

Right, but again we have to support CSS-like grammars with no ;, no :, no {... so really we can't depend on such things to tell us what is a property or a value, etc.

@joshgoebel
Copy link
Member Author

but it seems the attribute value is already being matched.

We don't match "values" generically, we match known types of patterns/values like 32px, "blah" (string), etc... take a custom name like "blah-blah"... now tell me is that a value, an attribute or a custom HTML tag name? There is no way to know... hence any generic rule matching IDENT as "string" is a non-starter...

@joshgoebel
Copy link
Member Author

Closing for lack of activity.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discuss/propose Proposal for a new feature/direction help welcome Could use help from community language
Projects
None yet
Development

No branches or pull requests

2 participants