Repository: matthewwithanm/python-markdownify

Issue:
div mis-converted
```
>>> MarkdownConverter().convert('<div>foo</div><div>bar<span>baz</span><span>meow</span></div>')
'foobarbazmeow'
```

Expected: `'foo  \nbarbazmeow'`

Discussion:
@mirabilos: Looking at the code, this is probably very hard to do because for this the *second* div has to lookbehind and see that the previous `text` did not end with a `\n\n` hard paragraph break.@mirabilos: Workaround (this relies on the fix for #92 to be applied):

```
>>> text = '<div>foo</div><div>bar<span>baz</span><span>meow</span></div>'
>>> html = bs4.BeautifulSoup(text, 'html.parser')
>>> for e in html.find_all('div'):
...     e.insert_before(html.new_tag('br'))
... 
>>> text = MarkdownConverter().convert_soup(html)
>>> text = re.sub('  \n  \n', '\n\n', text)
>>> text = re.sub(' *\n\n+', '\n\n', text).strip()
>>> text
'foo  \nbarbazmeow'
```

Maybe it helps someone.

For the sake of completeness, the following is my current complete example of how I clean up HTML from RSS feeds to post it, as Markdown, to Fediverse (called as `cleanup(post.x)` where `x` is title, summary, content, …), which includes a number of workarounds for bad input and limits of the conversion tools:

```
def _cleanup_tablish(tag):
    for e in tag.contents:
        if isinstance(e, bs4.element.NavigableString) and str(e).strip() == '':
            e.extract()
            return True
    return False

def _cleanup_table(top):
    tag = top
    while isinstance(tag, bs4.element.Tag) and \
      tag.name in ('table', 'tbody', 'tr', 'th', 'td'):
        while _cleanup_tablish(tag):
            pass
        have_tablish = False
        have_nontablish = False
        have_elts = 0
        for e in tag.contents:
            if isinstance(e, bs4.element.NavigableString):
                have_nontablish = True
            elif e.name in ('table', 'tbody', 'tr', 'th', 'td'):
                have_tablish = True
            else:
                have_nontablish = True
            have_elts = have_elts + 1
        if have_elts == 0:
            top.extract()
            return
        if have_nontablish:
            if have_tablish:
                # huh?
                return
            tag.name = 'div'
            tag.attrs.clear()
            e = tag.contents[0]
            if have_elts == 1 and isinstance(e, bs4.element.Tag) and \
              e.name in bs4.builder.HTMLTreeBuilder.block_elements:
                tag = e
            if tag != top:
                top.replace_with(tag)
            return
        if have_elts > 1:
            return
        tag = tag.contents[0]

_cleanup_traildots = re.compile('\\.\\.\\.$')
def cleanup(text):
    text = re.sub('\r+\n?', '\n', text)
    html = bs4.BeautifulSoup(text, 'html.parser', multi_valued_attributes=None)
    # remove <!-- comments -->
    for e in html.find_all(string=lambda e: isinstance(e, bs4.element.Comment)):
        e.extract()
    # flatten tables with only one cell (Goodreads)
    for e in html.find_all('table'):
        _cleanup_table(e)
    # expand shortened links
    for e in html.find_all('a', href=True, string=_cleanup_traildots):
        href = str(e['href'])
        if href.startswith(str(e.string).rstrip('.')):
            e.string.replace_with(href)
    # temporarily move <pre>s aside
    pres = []
    npres = 0
    for pre in html.find_all('pre'):
        pres.append(pre.replace_with(html.new_tag('rpre', num=npres)))
        npres = npres + 1
    # clean whitespace except in the extracted <pre>s
    text = str(html)
    text = re.sub(' *\n *', '\n', text)
    text = text.replace('\n', '\1')
    text = re.sub('\1\1\1+', '\n\n', text)
    text = re.sub('\1+ *', ' ', text).strip()
    text = re.sub('[\t ]+', ' ', text)
    # bring back the extracted <pre>s
    html = bs4.BeautifulSoup(text, 'html.parser')
    for pre in html.find_all('rpre'):
        pre.replace_with(pres[int(pre.attrs['num'])])
    # work around https://github.com/matthewwithanm/python-markdownify/issues/107
    for e in html.find_all('div'):
        e.insert_before(html.new_tag('br'))
    # convert and clean up
    text = MarkdownConverter(strip=['img']).convert_soup(html)
    text = re.sub('  \n  \n', '\n\n', '\n' + text + '\n')
    text = re.sub('(\n> )+\n', '\n> \n', '\n' + text + '\n')
    text = re.sub(' *\n\n+', '\n\n', text)
    return text.strip()
```@chrispy-snps: We're hitting this too. It is a difficult fix in the current architecture.@AlexVonB: Thanks for reporting this! Indeed it is quite hard to fix. Especially we would have to decide if divs always behave as paragraphs. If we do, we could just handle divs the save as p and it would be fixed. But I think this would break different stuff. I'm open to suggestions, tho.@mirabilos: On Sun, 24 Nov 2024, AlexVonB wrote:

>Especially we would have to decide if divs always behave as paragraphs.

Uhm… no need to decide it, there’s already a spec for that ;-)

Basically, a div forces the browser to begin at a new line,
thinking in terms like what text browsers such as lynx would use.
If the current position is already at a new line (e.g. because there
was a </p> before it, no need to do anything; otherwise, forcing a
linebreak is needed.

So they specifically _don’t_ behave like paragraphs, which also have
inter-paragraph spacing.

Hence, the example and expected text in the submission above.

bye,
//mirabilos
-- 
  "Using Lynx is like wearing a really good pair of shades: cuts out
   the glare and harmful UV (ultra-vanity), and you feel so-o-o COOL."
                                         -- Henry Nelson, March 1999
@chrispy-snps: We previously used a source-modifying workaround like @mirabilos posted. However, we recently switched to a subclass-based workaround:

```python
class CustomMarkdownConverter(markdownify.MarkdownConverter):
    """
    Create a custom MarkdownConverter that fixes some issues.
    """

    def convert_div(self, el, text, convert_as_inline):
        if convert_as_inline:
            return " " + text.strip() + " "
        else:
            return "\n\n" + text + "\n\n"
```

Our application prefers treating `<div>` boundaries as paragraph breaks, but you can modify it to use Markdown line breaks if preferred.

Solution patch:
diff --git a/markdownify/__init__.py b/markdownify/__init__.py
index 93a889f..9e4c99f 100644
--- a/markdownify/__init__.py
+++ b/markdownify/__init__.py
@@ -1,4 +1,4 @@
-from bs4 import BeautifulSoup, NavigableString, Comment, Doctype
+from bs4 import BeautifulSoup, Comment, Doctype, NavigableString, Tag
 from textwrap import fill
 import re
 import six
@@ -79,6 +79,7 @@ def should_remove_whitespace_inside(el):
     if html_heading_re.match(el.name) is not None:
         return True
     return el.name in ('p', 'blockquote',
+                       'article', 'div', 'section',
                        'ol', 'ul', 'li',
                        'table', 'thead', 'tbody', 'tfoot',
                        'tr', 'td', 'th')
@@ -89,6 +90,41 @@ def should_remove_whitespace_outside(el):
     return should_remove_whitespace_inside(el) or (el and el.name == 'pre')
 
 
+def _is_block_content_element(el):
+    """
+    In a block context, returns:
+
+    - True for content elements (tags and non-whitespace text)
+    - False for non-content elements (whitespace text, comments, doctypes)
+    """
+    if isinstance(el, Tag):
+        return True
+    elif isinstance(el, (Comment, Doctype)):
+        return False  # (subclasses of NavigableString, must test first)
+    elif isinstance(el, NavigableString):
+        return el.strip() != ''
+    else:
+        return False
+
+
+def _prev_block_content_sibling(el):
+    """Returns the first previous sibling that is a content element, else None."""
+    while el is not None:
+        el = el.previous_sibling
+        if _is_block_content_element(el):
+            return el
+    return None
+
+
+def _next_block_content_sibling(el):
+    """Returns the first next sibling that is a content element, else None."""
+    while el is not None:
+        el = el.next_sibling
+        if _is_block_content_element(el):
+            return el
+    return None
+
+
 class MarkdownConverter(object):
     class DefaultOptions:
         autolinks = True
@@ -143,29 +179,38 @@ def process_tag(self, node, convert_as_inline):
             or node.name in ['td', 'th']  # table cells
         )
 
-        # Remove whitespace-only textnodes just before, after or
-        # inside block-level elements.
+        # Collect child elements to process, ignoring whitespace-only text elements
+        # adjacent to the inner/outer boundaries of block elements.
         should_remove_inside = should_remove_whitespace_inside(node)
-        for el in node.children:
-            # Only extract (remove) whitespace-only text node if any of the
-            # conditions is true:
-            # - el is the first element in its parent (block-level)
-            # - el is the last element in its parent (block-level)
-            # - el is adjacent to a block-level node
-            can_extract = (should_remove_inside and (not el.previous_sibling
-                                                     or not el.next_sibling)
-                           or should_remove_whitespace_outside(el.previous_sibling)
-                           or should_remove_whitespace_outside(el.next_sibling))
-            if (isinstance(el, NavigableString)
-                    and six.text_type(el).strip() == ''
-                    and can_extract):
-                el.extract()
 
-        # Convert the children first
-        for el in node.children:
-            if isinstance(el, Comment) or isinstance(el, Doctype):
-                continue
+        def _can_ignore(el):
+            if isinstance(el, Tag):
+                # Tags are always processed.
+                return False
+            elif isinstance(el, (Comment, Doctype)):
+                # Comment and Doctype elements are always ignored.
+                # (subclasses of NavigableString, must test first)
+                return True
             elif isinstance(el, NavigableString):
+                if six.text_type(el).strip() != '':
+                    # Non-whitespace text nodes are always processed.
+                    return False
+                elif should_remove_inside and (not el.previous_sibling or not el.next_sibling):
+                    # Inside block elements (excluding <pre>), ignore adjacent whitespace elements.
+                    return True
+                elif should_remove_whitespace_outside(el.previous_sibling) or should_remove_whitespace_outside(el.next_sibling):
+                    # Outside block elements (including <pre>), ignore adjacent whitespace elements.
+                    return True
+                else:
+                    return False
+            else:
+                raise ValueError('Unexpected element type: %s' % type(el))
+
+        children_to_convert = [child for child in node.children if not _can_ignore(child)]
+
+        # Convert the children first
+        for el in children_to_convert:
+            if isinstance(el, NavigableString):
                 text += self.process_text(el)
             else:
                 text_strip = text.rstrip('\n')
@@ -337,6 +382,16 @@ def convert_code(self, el, text, convert_as_inline):
 
     convert_del = abstract_inline_conversion(lambda self: '~~')
 
+    def convert_div(self, el, text, convert_as_inline):
+        if convert_as_inline:
+            return ' ' + text.strip() + ' '
+        text = text.strip()
+        return '\n\n%s\n\n' % text if text else ''
+
+    convert_article = convert_div
+
+    convert_section = convert_div
+
     convert_em = abstract_inline_conversion(lambda self: self.options['strong_em_symbol'])
 
     convert_kbd = convert_code
@@ -415,7 +470,8 @@ def convert_list(self, el, text, convert_as_inline):
 
         nested = False
         before_paragraph = False
-        if el.next_sibling and el.next_sibling.name not in ['ul', 'ol']:
+        next_sibling = _next_block_content_sibling(el)
+        if next_sibling and next_sibling.name not in ['ul', 'ol']:
             before_paragraph = True
         while el:
             if el.name == 'li':
@@ -539,6 +595,7 @@ def convert_th(self, el, text, convert_as_inline):
 
     def convert_tr(self, el, text, convert_as_inline):
         cells = el.find_all(['td', 'th'])
+        is_first_row = el.find_previous_sibling() is None
         is_headrow = (
             all([cell.name == 'th' for cell in cells])
             or (el.parent.name == 'thead'
@@ -546,15 +603,15 @@ def convert_tr(self, el, text, convert_as_inline):
                 and len(el.parent.find_all('tr')) == 1)
         )
         is_head_row_missing = (
-            (not el.previous_sibling and not el.parent.name == 'tbody')
-            or (not el.previous_sibling and el.parent.name == 'tbody' and len(el.parent.parent.find_all(['thead'])) < 1)
+            (is_first_row and not el.parent.name == 'tbody')
+            or (is_first_row and el.parent.name == 'tbody' and len(el.parent.parent.find_all(['thead'])) < 1)
         )
         overline = ''
         underline = ''
         if ((is_headrow
              or (is_head_row_missing
                  and self.options['table_infer_header']))
-                and not el.previous_sibling):
+                and is_first_row):
             # first row and:
             # - is headline or
             # - headline is missing and header inference is enabled
@@ -568,10 +625,10 @@ def convert_tr(self, el, text, convert_as_inline):
             underline += '| ' + ' | '.join(['---'] * full_colspan) + ' |' + '\n'
         elif ((is_head_row_missing
                and not self.options['table_infer_header'])
-              or (not el.previous_sibling
+              or (is_first_row
                   and (el.parent.name == 'table'
                        or (el.parent.name == 'tbody'
-                           and not el.parent.previous_sibling)))):
+                           and not el.parent.find_previous_sibling())))):
             # headline is missing and header inference is disabled or:
             # first row, not headline, and:
             #  - the parent is table or


Test patch:
diff --git a/tests/test_basic.py b/tests/test_basic.py
index 584adb9..9be524e 100644
--- a/tests/test_basic.py
+++ b/tests/test_basic.py
@@ -6,7 +6,7 @@ def test_single_tag():
 
 
 def test_soup():
-    assert md('<div><span>Hello</div></span>') == 'Hello'
+    assert md('<div><span>Hello</div></span>') == '\n\nHello\n\n'
 
 
 def test_whitespace():
diff --git a/tests/test_conversions.py b/tests/test_conversions.py
index 1367006..1739cb9 100644
--- a/tests/test_conversions.py
+++ b/tests/test_conversions.py
@@ -114,8 +114,19 @@ def test_del():
     inline_tests('del', '~~')
 
 
-def test_div():
-    assert md('Hello</div> World') == 'Hello World'
+def test_div_section_article():
+    for tag in ['div', 'section', 'article']:
+        assert md(f'<{tag}>456</{tag}>') == '\n\n456\n\n'
+        assert md(f'123<{tag}>456</{tag}>789') == '123\n\n456\n\n789'
+        assert md(f'123<{tag}>\n 456 \n</{tag}>789') == '123\n\n456\n\n789'
+        assert md(f'123<{tag}><p>456</p></{tag}>789') == '123\n\n456\n\n789'
+        assert md(f'123<{tag}>\n<p>456</p>\n</{tag}>789') == '123\n\n456\n\n789'
+        assert md(f'123<{tag}><pre>4 5 6</pre></{tag}>789') == '123\n\n```\n4 5 6\n```\n\n789'
+        assert md(f'123<{tag}>\n<pre>4 5 6</pre>\n</{tag}>789') == '123\n\n```\n4 5 6\n```\n\n789'
+        assert md(f'123<{tag}>4\n5\n6</{tag}>789') == '123\n\n4\n5\n6\n\n789'
+        assert md(f'123<{tag}>\n4\n5\n6\n</{tag}>789') == '123\n\n4\n5\n6\n\n789'
+        assert md(f'123<{tag}>\n<p>\n4\n5\n6\n</p>\n</{tag}>789') == '123\n\n4\n5\n6\n\n789'
+        assert md(f'<{tag}><h1>title</h1>body</{{tag}}>', heading_style=ATX) == '\n\n# title\n\nbody\n\n'
 
 
 def test_em():