-
Notifications
You must be signed in to change notification settings - Fork 9
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
Fix #57: Compile selectors at css parsing time. #58
Conversation
lib/extended-css-parser.js
Outdated
var match, matchPos, matched; | ||
nextIndex++; // Move the pointer to the start of style declaration. | ||
while (true) { | ||
untilClosingBracket: { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The code is too messy. Extract untilClosingBracket
into a function.
lib/extended-css-parser.js
Outdated
var styles = Object.create(null); | ||
var match, matchPos, matched; | ||
nextIndex++; // Move the pointer to the start of style declaration. | ||
while (true) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Avoid while (true)
loops at any cost. There's always a chance of an infinite loop.
If you can't find a better way, use for (i < SOME_HUGE_NUMBER)
and throw an error if you didn't get the result in the end.
lib/extended-css-parser.js
Outdated
match = reNonWhitespace.exec(cssText); | ||
var styleMap = this.parseNextStyle(); | ||
|
||
results.push({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Empty styleMap is not handled
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Worth a unit-test mbe?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Empty styleMap is not handled
Do we need any special treatment?
Worth a unit-test mbe?
For what?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need any special treatment?
Nvm, I thought that empty style signals about a mistake. I can see that it's not the case actually.
lib/extended-css-parser.js
Outdated
match = reNonWhitespace.exec(cssText); | ||
var styleMap = this.parseNextStyle(); | ||
|
||
results.push({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need any special treatment?
Nvm, I thought that empty style signals about a mistake. I can see that it's not the case actually.
Merge in EXTENSIONS/extended-css from feature/AG-2144 to master * commit '7b2ba4660f2402fc064838a187191c6459373189': few tiny edits fix readme typo handle 'null' and 'undefined' property values set as string refactor utils, add matcherUtils tests, update readme refactor parseRawPropChain handle null as value while comparing, add extra tests for regexp chain props implement matches-property pseudo
No description provided.