remove superfluous leading/trailing whitespace #181
Conversation
Signed-off-by: chrispy <[email protected]>
|
@AlexVonB - the unit tests no longer validate the expected leading newlines for each element. I am not sure I like this. I am thinking of adding an option to control whether the final document-level newline stripping is performed, so that it can be disabled in the unit tests and the leading newlines can be checked. What do you think? |
chrispy-snps
force-pushed
the
chrispy/remove-extra-whitespace
branch
from
80239dc to
dd90a52
Compare
Signed-off-by: chrispy <[email protected]>
chrispy-snps
force-pushed
the
chrispy/remove-extra-whitespace
branch
from
dd90a52 to
dd251c6
Compare
|
After more thought, I made the document-stripping behavior configurable, then reverted the unit tests to their original behavior of testing leading/trailing newlines. |
There was a problem hiding this comment.
So currently the conversion of <html><body><p>foo<p></body></html> returns \n\nfoo\n\n, correct? Then, removing the first two newlines seems sensible.
Be aware that making LSTRIP the default changes the current behavior. We should think about increasing the minor version instead just the patch (makes me think about going out of 0.x some time? :D). Maybe even keeping None as default could be a thing. Let me know what you think!
Thanks for your work, again! :)
|
@AlexVonB - yep, current behavior is newlines on both sides, which never really felt right to me: md("<html><body><p>foo<p></body></html>")
# '\n\nfoo\n\n'Given your openness to having a default of |
…mon test utils file Signed-off-by: chrispy <[email protected]>
chrispy-snps
force-pushed
the
chrispy/remove-extra-whitespace
branch
from
c9470e0 to
1586f98
Compare
|
@AlexVonB - can you give one final sanity check review please? |
|
Looks good! Thanks again! |
Removes superfluous leading/trailing whitespace (EDITED to final behavior):
strip_documentconfiguration option, which defaults toSTRIP.strip_document == Noneso that leading/trailing separation newline behavior is tested.<p>,<caption>, and<figcaption>elements now always return the minimum required leading/trailing newlines, even when their child content contributes additional newlines.<figcaption>is renamed and moved.In the
convert__document_()function, I decided not to remove trailing newlines by default (LSTRIP) so that content can be appended to the result without ambiguity. However, removing both leading and trailing newlines is also worth considering.This is proactive cleanup that will hopefully make #162 easier.