You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
This code section near the bottom of markdown.treeprocessors raises AttributeError if pre[0].text is None.
# Clean up extra empty lines at end of code blocks.pres=root.iter('pre')
forpreinpres:
iflen(pre) andpre[0].tag=='code':
pre[0].text=util.AtomicString(pre[0].text.rstrip() +'\n')
The check should be if len(pre) and pre[0].tag == 'code' and pre[0].text is not None or isinstance(pre[0].text, str), but that may be too strict.
I encountered this with a custom Block/Inline processor. The band-aid fix was just to ensure these elements had text.
Based on the information you provided, I don't understand how that results in an error. All we are doing is resigning pre[0].text to a new value, which is exactly the same thing you are doing in your band-aid. Could you please point to where this code is so I can better understand it in context?
Oh, I got it. pre[0].text.rstrip() raises an error if pre[0].text is None.
I believe this has not been an issue thus far as internally we always assign a string value to the text attribute of an element. The question is: do we want to require all extensions to do the same or not? If not, then we should apply the suggested fix. If so, then we can leave it alone.
Finally, the relevant code is here for future reference.
When text is None , if text evaluates to False and the serializer simply skips writing out the text content and moves on to child elements (if any) and the closing tag. It makes sense for an element with text of None to simply be empty (no different that text being an empty string). Therefore, there is no reason to require an element's text attribute to be of type str.
Finally, the question remains whether the prettifier needs to take any action on an empty code block. In other words, do we want
<pre><code></code></pre>
or
<pre><code></code></pre>
If the former, then the suggested fix will work fine. If the latter, then we need a slightly more sophisticated fix. Checking Balemark (compare this with this) is looks like the first option is acceptable.
Given the above, a PR is welcome adding and pre[0].text is not None to the check.
Minor note, this also affects non-empty blocks, i.e. blocks with children but no text, such as <pre><code><span>x</span></code></pre>. But I don't think that changes anything here.
Activity
[-]PrettifyTreeprocessor TypeError[/-][+]PrettifyTreeprocessor AttributeError[/+]waylan commentedon May 25, 2022
Based on the information you provided, I don't understand how that results in an error. All we are doing is resigning
pre[0].textto a new value, which is exactly the same thing you are doing in your band-aid. Could you please point to where this code is so I can better understand it in context?waylan commentedon May 25, 2022
Oh, I got it.
pre[0].text.rstrip()raises an error ifpre[0].textisNone.I believe this has not been an issue thus far as internally we always assign a string value to the
textattribute of an element. The question is: do we want to require all extensions to do the same or not? If not, then we should apply the suggested fix. If so, then we can leave it alone.Finally, the relevant code is here for future reference.
fourpoints commentedon May 25, 2022
By default
ET.Element(tag).textisNone. But you could add a requirement that it is a string. Thanks for linking to the code.waylan commentedon May 25, 2022
I was curious how the serializer handles a
Nonevalue so I checked the code:markdown/markdown/serializers.py
Lines 160 to 168 in a2e4788
When
textisNone,if textevaluates toFalseand the serializer simply skips writing out the text content and moves on to child elements (if any) and the closing tag. It makes sense for an element withtextofNoneto simply be empty (no different thattextbeing an empty string). Therefore, there is no reason to require an element'stextattribute to be of typestr.Finally, the question remains whether the prettifier needs to take any action on an empty code block. In other words, do we want
or
If the former, then the suggested fix will work fine. If the latter, then we need a slightly more sophisticated fix. Checking Balemark (compare this with this) is looks like the first option is acceptable.
Given the above, a PR is welcome adding
and pre[0].text is not Noneto the check.fourpoints commentedon May 25, 2022
Thanks, I'll make a PR later :)
fourpoints commentedon May 25, 2022
Thanks for your help! 👍
Minor note, this also affects non-empty blocks, i.e. blocks with children but no text, such as
<pre><code><span>x</span></code></pre>. But I don't think that changes anything here.Add None check to PrettifyTreeprocessor