noviluni commented 3 years ago

This is just an idea I had that I would like to discuss.

I've seen that the requests-html library adds a links() method to extract all the URLs in the HTML. In parsel you can easily extract the links by using css() or xpath(), but there could be some logic that could be simplified by creating a links() method (see code).

A possible use would be to use it in conjunction with scrapy.Response.follow_all() to easily crawl all URLs in the HTML.

Of course, this should be refactored, properly documented, and tested. Or even converted to a generator. We could also exclude other URL patterns like tel: or even add a method parameter to filter the URLS (by a regex pattern like we do in scrapy.sitemap.SitemapSpider?) or implement an option to get the absolute links.

So I would like to know what you think about this, if you think it's useful or not and if it should be implemented or not (or alternatively, if it should be implemented directly into the scrapy.selector.Selector) :slightly_smiling_face:

Resolves #26

codecov[bot] commented 3 years ago

Codecov Report

Merging #216 (9663dd1) into master (80509e4) will decrease coverage by 0.33%. The diff coverage is 50.00%.

:exclamation: Current head 9663dd1 differs from pull request most recent head f0a8c56. Consider uploading reports for the commit f0a8c56 to get more accurate results

@@             Coverage Diff             @@
##            master     #216      +/-   ##
===========================================
- Coverage   100.00%   99.66%   -0.34%     
===========================================
  Files            5        5              
  Lines          298      300       +2     
  Branches        51       52       +1     
===========================================
+ Hits           298      299       +1     
- Misses           0        1       +1

Impacted Files	Coverage Δ
parsel/selector.py	`99.36% <50.00%> (-0.64%)`	:arrow_down:

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 80509e4...f0a8c56. Read the comment docs.

Gallaecio commented 3 years ago

See https://github.com/scrapy/parsel/issues/26 for some background discussion that goes back to a Scrapy issue.

scrapy / parsel

PoC/discussion: add links() #216

Codecov Report