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

Fix :nth-*(an+b) pseudo-classes selectors #60

Merged
merged 8 commits into from
Sep 8, 2016
Merged

Fix :nth-*(an+b) pseudo-classes selectors #60

merged 8 commits into from
Sep 8, 2016

Conversation

redapple
Copy link
Contributor

@redapple redapple commented Jul 12, 2016

This is a continuation of #42 but following https://www.w3.org/TR/css3-selectors/#structural-pseudos definitions more closely, that is, counting preceding-sibling:: or following-sibling:: explicitly, instead of using context-dependent position():

:nth-child() pseudo-class
The :nth-child(an+b) pseudo-class notation represents an element that has an+b-1 siblings before it in the document tree

In fact, this was suggested in sparklemotion/nokogiri#707 (comment).

It fixes #12, #15, #46.

Another change is xpath_descendant_combinator which now translates to /descendant (instead of /descendant-or-self), which looks more inline with Descendant combinator:

A selector of the form "A B" represents an element B that is an arbitrary descendant of some ancestor element A.

The general case of h2 ~ *:nth-of-type(2) is not fixed by this PR. It looks hard (or impossible) to achieve with XPath 1.0 alone, but it may not be that common case.

@codecov-io
Copy link

codecov-io commented Jul 12, 2016

Current coverage is 95.00%

Merging #60 into master will decrease coverage by 0.04%

@@             master        #60   diff @@
==========================================
  Files             3          3          
  Lines           708        701     -7   
  Methods           0          0          
  Messages          0          0          
  Branches        114        113     -1   
==========================================
- Hits            673        666     -7   
- Misses           19         20     +1   
+ Partials         16         15     -1   

Powered by Codecov. Last updated by b6ba4ff...7fdcf08

@redapple redapple changed the title Fix :nth-*(an+b) pseudo-classes selectors for negative a's (2nd take) [WIP] Fix :nth-*(an+b) pseudo-classes selectors for negative a's (2nd take) Jul 12, 2016
@redapple redapple changed the title [WIP] Fix :nth-*(an+b) pseudo-classes selectors for negative a's (2nd take) [WIP] Fix :nth-*(an+b) pseudo-classes selectors Jul 12, 2016
@redapple redapple changed the title [WIP] Fix :nth-*(an+b) pseudo-classes selectors Fix :nth-*(an+b) pseudo-classes selectors Jul 12, 2016
redapple referenced this pull request in sjp/selectr Aug 29, 2016
@kmike
Copy link
Member

kmike commented Sep 8, 2016

Hi @redapple! It seems we'll have hard time finding anyone to review this pull request properly. I'm merging it anyways because

  • existing tests pass;
  • the PR fixes several issues;
  • the approach looks in line with CSS selector specs;
  • code looks good.

@kmike kmike merged commit 4caba2d into scrapy:master Sep 8, 2016
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.

:nth-child incorrect with + or ~
3 participants