hoextdown icon indicating copy to clipboard operation
hoextdown copied to clipboard

"Loose" list items are sticky - all following list items become loose

Open jasharpe opened this issue 9 years ago • 5 comments

Input:

Loose should not be sticky:

* foo
* bar

  more bar

* baz
* bat

Expected:

<p>Loose should not be sticky:</p>

<ul>
<li>foo</li>
<li>
<p>bar</p>
<p>more bar</p>
</li>
<li>baz</li>
<li>bat</li>
</ul>

Actual:

<p>Loose should not be sticky:</p>

<ul>
<li>foo</li>
<li>
<p>bar</p>
<p>more bar</p>
</li>
<li>
<p>baz</p>
</li>
<li>
<p>bat>/p>
</li>
</ul>

This is incorrect and looks horrible when rendered in HTML, since the first two items and close together, and the last two items are far apart, even though they're both tight in the markdown.

jasharpe avatar Jul 08 '15 15:07 jasharpe

This issue is affecting my project as well. I filed an issue against Hoedown, but it's only in maintenance mode atm. It was suggested someone here might be interested in taking a look.

We use hoextdown internally here at Google for a lot of our internal documentation, and this styling issue is a bit of an eyesore.

joeltine avatar Jul 08 '15 18:07 joeltine

A solution would be to start parsing the list again from the start whenever the list becomes loose.

It's not very hard, but it does require some refactoring, but the list parsing code is already complex and I'm not touching it...

mildsunrise avatar Jul 08 '15 19:07 mildsunrise

Another idea is to not render list items inside rndr_listitem, but cache information inside hoedown_html_renderer_state. The whole list is can be rendered in rndr_list instead.

Something like…

typedef struct hoedown_html_listitem_data {
    hoedown_buffer content;
    hoedown_buffer attr;
    hoedown_list_flags list_flags;
    hoedown_html_flags html_flags
} hoedown_html_listitem_data;

typedef struct hoedown_html_listitem_data {
    hoedown_buffer content;
    hoedown_buffer attr;
    hoedown_list_flags list_flags;
    hoedown_html_flags html_flags
} hoedown_html_listitem_data;

typedef struct hoedown_list {
    void *items;
    size_t size;
} hoedown_list;

struct hoedown_html_renderer_state {
    // ...
   hoedown_stack listitem_data_stack;
};

The stack is needed since lists can be nested. A new hoedown_list is pushed into the stack when a list opens. Information of list items is collected into the hoedown_list. Finally in rndr_list the hoedown_list is popped, and list item rendered based on information inside.

This approach will require an extra renderer callback to indicate the begin of a new list (to push the new list). Performance would probably need to be considered, too.

uranusjr avatar Jul 08 '15 20:07 uranusjr

Hoedown 4.0 does an hybrid approach, it caches the contents of every list item, then at the end it calls the listitem callback as usual for every list item, then the list callback. BTW, we don't have hoedown_list in 3.0, but we do have hoedown_stack.

mildsunrise avatar Jul 08 '15 20:07 mildsunrise

Note: https://spec.commonmark.org/0.29/#lists writes that

A list is loose if any of its constituent list items are separated by blank lines, or if any of its constituent list items directly contain two block-level elements with a blank line between them. Otherwise a list is tight.

I'm not sure whether Hoextdown is aiming for commonmark compatibility, but in that case the expected behavior in the initial comment should be loose rendering for all items. To me this makes a lot more sense, because particularly with a blank line between items, it's hard to attribute the looseness to one specific item. Comments here about repeated or delayed processing seem to be aiming for a consistent loose rendering, too, the way I understand them.

gagern avatar Jun 25 '20 08:06 gagern