scrapy / parsel

Parsel lets you extract data from XML/HTML documents using XPath or CSS selectors
BSD 3-Clause "New" or "Revised" License
1.11k stars 137 forks source link

Issue #249 - Add strip to get() and getall() #260

Open felipeboffnunes opened 1 year ago

felipeboffnunes commented 1 year ago

Resolves #249

codecov[bot] commented 1 year ago

Codecov Report

Merging #260 (7ce8789) into master (1913fb7) will not change coverage. The diff coverage is n/a.

:exclamation: Current head 7ce8789 differs from pull request most recent head b044b76. Consider uploading reports for the commit b044b76 to get more accurate results

@@            Coverage Diff            @@
##            master      #260   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            4         4           
  Lines          138       138           
  Branches        29        29           
=========================================
  Hits           138       138           

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

kmike commented 1 year ago

Hey! Is there a use case for this feature if https://github.com/scrapy/parsel/pull/127 is merged, do you have some examples?

I'm asking because sel.get(text=True) would also be stripping text by default, so strip=True won't be useful with text argument. It may be also hard to support strip=False, text=True. And I wonder if there are real-world cases where you want strip=True, but don't want text=True.

felipeboffnunes commented 1 year ago

@kmike By the time I saw #127 I had most of the bulk of the strip logic coded. I do0n't think there is a common use case for this PR compared to 127 besides maybe being a more straight forward pre processing step of the text where only one thing happens (strip). Either way I felt that #127 has been around for a good while and not finished, so a simpler alternative was presented just to fix the issue #249 I guess we can close this one, mention #127 in #249 and try to close the features presented there