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

Propose an alternative way to solve regression #38

Merged
merged 3 commits into from
Nov 24, 2015
Merged

Propose an alternative way to solve regression #38

merged 3 commits into from
Nov 24, 2015

Conversation

Delgan
Copy link
Collaborator

@Delgan Delgan commented Nov 24, 2015

This is a continuation of my previous pull request.

I am glad I could help you.
But I said my changes looked ugly, so I tried to improve it.

This commit looks more elegant and efficient than the previous one which added complexity to the code and parameters while trying to make a special descendant selector "flexible".
I think the best is simply to add a new "flexible descendant" selector, so I created a new type of descendant token which accepts element itself. Then, I replace classic descendant within the rule if conditions are fulfilled.

Create a new type of descendant token which accepts element itself.
This looks more elegant and efficient than the previous commit which added complexity to the code and parameters.

This is only used while querying an array of elements.
Could throw an error if a query is just ":scope" for example, because of
`t[1]` while tokens array contains only one element.
fb55 added a commit that referenced this pull request Nov 24, 2015
Propose an alternative way to solve regression
@fb55 fb55 merged commit 0d812a0 into fb55:master Nov 24, 2015
@fb55
Copy link
Owner

fb55 commented Nov 24, 2015

@Delgan Haha, nice improvement :) I would call the new token type _flexibleDescendant, to underline it's only internal, and there is still a console.log left. I've given you push access to the repo, if you want to fix it.

@Delgan
Copy link
Collaborator Author

Delgan commented Nov 24, 2015

@fb55 Oops, sorry about that, it is fixed now, thank you.

fb55 added a commit that referenced this pull request Oct 21, 2018
Propose an alternative way to solve regression
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.

2 participants