Skip to content

Conversation

arthurdarcet
Copy link
Contributor

My use case is the following:

  • parse a selector group
  • filter out some rules I don't want
  • export the remaining selectors back to CSS

I have added a css method on the Selector class (and on each "sub"-selectors), to be able to export them back to CSS

@codecov-io
Copy link

codecov-io commented Mar 7, 2017

Codecov Report

Merging #75 into master will increase coverage by 0.03%.
The diff coverage is 91.3%.

@@            Coverage Diff             @@
##           master      #75      +/-   ##
==========================================
+ Coverage   95.02%   95.05%   +0.03%     
==========================================
  Files           3        3              
  Lines         703      748      +45     
  Branches      114      124      +10     
==========================================
+ Hits          668      711      +43     
- Misses         20       21       +1     
- Partials       15       16       +1
Impacted Files Coverage Δ
cssselect/xpath.py 94.86% <60%> (+0.03%)
cssselect/parser.py 95.12% <95.12%> (+0.02%)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3036cba...08b0e47. Read the comment docs.

@redapple
Copy link
Contributor

Hi @arthurdarcet , sorry for the delayed feedback.
I quite like the idea, makes sense to me.
I wonder about the .css() name though. I was thinking about .canonical() perhaps. No strong opinion though.

if selector.value is None:
value = None
elif self.lower_case_attribute_values:
value = selector.value.value.lower()
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this fixing a bug? I don't quite get this changing to selector.value.value.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it's the symmetrical change for this: https://github.com/scrapy/cssselect/pull/75/files/08b0e472175197d4887612cd90d74815ab10d90a#diff-adc3ae8f2cf8b1931771d84ca7af6275R596

needed because i had to distinguish between different types of values here: https://github.com/scrapy/cssselect/pull/75/files/08b0e472175197d4887612cd90d74815ab10d90a#diff-adc3ae8f2cf8b1931771d84ca7af6275R651 (strings have to be serialised with quotes, number have to skip them, …)

I'm not sure I remember why I needed

@arthurdarcet
Copy link
Contributor Author

Your call for the method name, you're right canonical is closer to what it does but it feels a bit too abstract to me. canonical_text or canonical_css?

@codecov
Copy link

codecov bot commented Feb 11, 2019

Codecov Report

Merging #75 into master will increase coverage by 0.32%.
The diff coverage is 96.15%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #75      +/-   ##
==========================================
+ Coverage   95.25%   95.57%   +0.32%     
==========================================
  Files           2        2              
  Lines         695      746      +51     
  Branches      113      125      +12     
==========================================
+ Hits          662      713      +51     
  Misses         19       19              
  Partials       14       14
Impacted Files Coverage Δ
cssselect/parser.py 96.03% <100%> (+0.47%) ⬆️
cssselect/xpath.py 94.86% <60%> (+0.03%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cb7a7e2...8d0ff3e. Read the comment docs.

@dangra
Copy link
Member

dangra commented Jul 9, 2019

@Gallaecio I think this could be useful and almost ready. Could you take a look? thanks

@Gallaecio
Copy link
Member

It looks good to me. However, I wonder if, now that #87 has been merged, we need to take it into account here as well.

Copy link
Member

@Gallaecio Gallaecio left a comment

Choose a reason for hiding this comment

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

Nevermind, :scope > * works just fine.

@kmike
Copy link
Member

kmike commented Aug 8, 2019

Thanks @arthurdarcet! And thanks @redapple, @dangra and @Gallaecio for reviews.

@kmike kmike merged commit 8f5c7a8 into scrapy:master Aug 8, 2019
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.

6 participants