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

adding matches pseudo-class #186

Closed
wants to merge 6 commits into from

Conversation

chrisdavidmills
Copy link
Contributor

@@ -272,6 +272,14 @@
],
"status": "standard"
},
":matches()": {
"syntax": ":matches( selector[, selector]* )",
Copy link
Contributor

@lahmatiy lahmatiy Feb 28, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

selector[, selector]* -> <selector>#
However, it takes a <selector-list> (https://www.w3.org/TR/selectors-4/#matches), but this syntax is not defined yet. Grammar can be taken here: https://www.w3.org/TR/selectors-4/#grammar

@chrisdavidmills
Copy link
Contributor Author

chrisdavidmills commented Mar 1, 2018

@lahmatiy So, I tried adding the necessary syntax definitions. That was a serious rabbit hole to fall down ;-)

I took all the syntax definitions straight from the spec, except for:

  • <ident-token>
  • <hash-token>
  • <function-token>
  • <string-token>
  • <any-value>

These are just described and not actually shown, so I'm not 100% sure if I've got these right.

@@ -272,6 +272,14 @@
],
"status": "standard"
},
":matches()": {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be :matches.

@@ -233,6 +260,9 @@
"frequency-percentage": {
"syntax": "<frequency> | <percentage>"
},
"function-token": {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

function-token is a token type and out of scope of this dict.
Btw, it has another syntax

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So I should remove these then...? How should this be represented then in our data? Some kind of link to the spec definition of token types?

The syntax definition I included was basically a guess based on my knowledge of related syntax, but I didn't really know what I was doing.

Same comment for the other token types.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Those tokens are defined in CSS Syntax Module Level 3 and shouldn't be presented in syntax.json. Because they are something like a generic, as <string> or <number> but a bit low-level.

@@ -254,6 +284,9 @@
"grid-line": {
"syntax": "auto | <custom-ident> | [ <integer> && <custom-ident>? ] | [ span && [ <integer> || <custom-ident> ] ]"
},
"hash-token": {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hash-token is a token type and out of scope of this dict.
Btw, it has another syntax

"id-selector": {
"syntax": "<hash-token>"
},
"ident-token": {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ident-token is a token type and out of scope of this dict.
Btw, it has another syntax

@@ -716,6 +767,12 @@
"step-timing-function": {
"syntax": "step-start | step-end | steps(<integer>[, [ start | end ] ]?)"
},
"string-token": {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

string-token is a token type and out of scope of this dict.
Btw, it has another syntax

@@ -11,12 +11,24 @@
"animateable-feature": {
"syntax": "scroll-position | contents | <custom-ident>"
},
"any-value": {
"syntax": "[!]+"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know what means such syntax. Could you explain?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Heh, I was guessing what it should possibly be, based on reading this defintion https://drafts.csswg.org/css-syntax-3/#typedef-any-value

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, well, that' diffidently incorrect. [!]+ means one or more exclamation marks and nothing else ;)
<any-value> can't be represented with such syntax, since it means any valid CSS value. Actually it's a generic and should be excluded from syntax.json.

Copy link
Contributor Author

@chrisdavidmills chrisdavidmills Mar 6, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you again ;-)

I have opened up a separate issue to track this: #190

@@ -11,12 +11,24 @@
"animateable-feature": {
"syntax": "scroll-position | contents | <custom-ident>"
},
"any-value": {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe it's a generic that out of scope of this dict

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK. Thanks for your continued help.

@chrisdavidmills
Copy link
Contributor Author

@teoli2003 can I get your advice on how we can make progress on this one?

@pkuczynski
Copy link
Contributor

@lahmatiy if we remove all you listed above, would it be ready to go?

@ExE-Boss
Copy link
Contributor

This needs rebasing

@ExE-Boss
Copy link
Contributor

ExE-Boss commented Sep 7, 2018

I got fed up with waiting and opened #284.

@chrisdavidmills
Copy link
Contributor Author

I'm really glad you did — you've actually come up with a really simple solution to this, whereas my attempt was horribly overcomplicated and we got stalled on it.

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