Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Some elements turning into None in html output with md_in_html #1049

Closed
greenape opened this issue Oct 21, 2020 · 7 comments · Fixed by #1050
Closed

Some elements turning into None in html output with md_in_html #1049

greenape opened this issue Oct 21, 2020 · 7 comments · Fixed by #1050
Labels
bug Bug report. confirmed Confirmed bug report or approved feature request. extension Related to one or more of the included extensions.

Comments

@greenape
Copy link

import markdown
md = markdown.Markdown(extensions=["markdown.extensions.md_in_html"])
test = """
<div class="output_area" markdown="1">
<table>
<thead><tr>
<th style="text-align:right"></th>
<th style="text-align:right">time</th>
<th style="text-align:right">amplitude</th>
</tr>
</thead>
<tbody>
<tr>
<td style="text-align:right">0</td>
<td style="text-align:right">0</td>
<td style="text-align:right">0</td>
</tr>
</tbody>
</table>
</div>
"""
print(md.convert(test))

Which gives me -

<div class="output_area">
<table>
<thead>None<tr>
<th style="text-align:right">None</th>
<th style="text-align:right">time</th>
<th style="text-align:right">amplitude</th>
</tr>
</thead>
<tbody>
<tr>
<td style="text-align:right">0</td>
<td style="text-align:right">0</td>
<td style="text-align:right">0</td>
</tr>
</tbody>
</table>
</div>

Rather than

<div class="output_area">
<table>
<thead><tr>
<th style="text-align:right"></th>
<th style="text-align:right">time</th>
<th style="text-align:right">amplitude</th>
</tr>
</thead>
<tbody>
<tr>
<td style="text-align:right">0</td>
<td style="text-align:right">0</td>
<td style="text-align:right">0</td>
</tr>
</tbody>
</table>
</div>

Which is what I'm expecting - note the extra None before the <tr> in <thead> and in the first header.

@facelessuser
Copy link
Collaborator

Yup, looks like another bug in the new HTML parser, unfortunately. Looks like we need a None check somewhere before inserting some text content.

@waylan waylan added bug Bug report. confirmed Confirmed bug report or approved feature request. extension Related to one or more of the included extensions. labels Oct 21, 2020
@facelessuser
Copy link
Collaborator

I think I may have a fix.

@waylan
Copy link
Member

waylan commented Oct 21, 2020

Here is a simpler case:

<div markdown="1">
<div></div>
</div>

Note that the text content of the second <div> is empty. Presumably, Element.text is None rather than an empty string as it should be. We should probably just address this in the serializer to avoid None in any output.

@waylan
Copy link
Member

waylan commented Oct 21, 2020

@facelessuser if you have a fix, go for it. Although we should probably do something with the serializer as well.

@facelessuser
Copy link
Collaborator

@waylan I'm not sure what you mean in regards to the serializer.

@facelessuser
Copy link
Collaborator

I've created pull #1050. We can discuss additional changes there if more than what I'm doing needs to be addressed.

@waylan
Copy link
Member

waylan commented Oct 21, 2020

@waylan I'm not sure what you mean in regards to the serializer.

My thinking was that there could be any number of other situations in which the text of an Element is None. We should never render that as None. However, I have since looked at the serializer and see that every call to write(text) is nested within a if text check, so the serializer already accounts for this.

It would seem that the issue is isolated to parsing the content of an Element as Markdown, which your fix addresses. So nevermind.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Bug report. confirmed Confirmed bug report or approved feature request. extension Related to one or more of the included extensions.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants