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

Handle interpolation in selector-nest-combinators #307

Merged
merged 3 commits into from Feb 2, 2019

Conversation

kristerkari
Copy link
Collaborator

Fixes #306

@coveralls
Copy link

coveralls commented Feb 1, 2019

Coverage Status

Coverage increased (+0.03%) to 92.244% when pulling 9ff7144 on bugfix/selector-nest-combinators-interpolation into 5032a3b on master.

}

// attribute, class, combinator, comment, id, nesting, pseudo, root, selector, string, tag, or universal
const chainingTypes = [
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I move these things on top, so that they're not inside a loop

@kristerkari
Copy link
Collaborator Author

@lxsymington maybe you want to have a look at the changes for my bugfix? :)

let message;

fullSelector.walk(node => {
if (node.value === "}") {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I added this because the selector parser can not properly parse interpolation, so the } was being reported on its own.

"universal"
];

const interpolationRe = /#{.+}$/;
Copy link
Contributor

Choose a reason for hiding this comment

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

Given that this is checking for the presence of any interpolation in the current selector, it's probably not particularly relevant, but would it generallly be better to use the non-greedy + operator? So that it would not match the whole of #{$var} .selector li #{$var-2}. https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/RegExp#quantifiers

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I agree, it's better to make it non-greedy.

Copy link
Contributor

@lxsymington lxsymington left a comment

Choose a reason for hiding this comment

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

Looks good to me! Just a non-blocking query about the regex

@kristerkari kristerkari merged commit cf5ac71 into master Feb 2, 2019
@kristerkari kristerkari deleted the bugfix/selector-nest-combinators-interpolation branch February 2, 2019 01:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants