Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.27%. Comparing base (91c567c) to head (bc83c56).
Report is 1 commits behind head on master.

@@           Coverage Diff           @@
##           master     #772   +/-   ##
=======================================
  Coverage   99.27%   99.27%           
=======================================
  Files          21       21           
  Lines        3587     3608   +21     
=======================================
+ Hits         3561     3582   +21     
  Misses         26       26           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Hi @unsleepy22, a few comments on the PR:

• I believe this fixes Preserve horizontal space in code blocks #553, could you please check?
• There is already a preserve_spaces parameter in htmlprocessing/handle_textnode, is it necessary to use a new argument across multiple functions?
• This behavior could actually become the default if there are no adverse effects, it would be easier for maintenance

Last, if you have several PRs in store about unrelated parts of the code it would be best to send them all at once, I don't plan to maintain the software on a day-to-day basis, we could both save time by grouping requests.

Hi @unsleepy22, a few comments on the PR:

• I believe this fixes Preserve horizontal space in code blocks #553, could you please check?
• There is already a preserve_spaces parameter in htmlprocessing/handle_textnode, is it necessary to use a new argument across multiple functions?
• This behavior could actually become the default if there are no adverse effects, it would be easier for maintenance

Last, if you have several PRs in store about unrelated parts of the code it would be best to send them all at once, I don't plan to maintain the software on a day-to-day basis, we could both save time by grouping requests.

Sorry for late response as I have been on vacation for a few days.

Yes, this fixes Preserve horizontal space in code blocks #553.
And also I added code to fix #769.

The preserve_spaces option does not work globally in [htmlprocessing/handle_textnode], and to keep backward-compatibility, I leave preserve_spaces to False by default.

Another change I'd like to make but needs discussion is adding another option like include_metadata_in_text to preserve/remove metadata dict string from text, but this may cause a bit confusion since we have 3 options regarding metadata, and, again, just for backward-compatibility. Another way to do this is change the default behavior of with_metadata option, which might introduce breaking changes. What do you think?

• If preserve_spaces is nearly always set to false, wouldn't it be easier to just remove the parameter and consider that space should be preserved in certain elements only like code blocks?
• I'm not sure about with_metadata, what do you want to achieve by doing this? Metadata is already added in TXT/MD format when with_metadata=True or is there a bug?

• If preserve_spaces is nearly always set to false, wouldn't it be easier to just remove the parameter and consider that space should be preserved in certain elements only like code blocks?
• I'm not sure about with_metadata, what do you want to achieve by doing this? Metadata is already added in TXT/MD format when with_metadata=True or is there a bug?

It's ok with me to make preserve_spaces default to True, I'll change my code a bit and fix a few UTs.

Currently if with_metadata is set to True, the metadata dict string will be prepended before text, what I want is to exclude metadata dict string from text and return within the Document.

OK then, you can change the code of preserve_spaces to avoid adding another parameter.

Concerning with_metadata this is expected result when people use extract(). To get metadata separately from the text you could just use bare_extraction(). The text is then in .raw_text, the title in .title etc.
If this is not enough please open an issue with an example of the output you want to get.