kjdev / hoextdown

Hoextdown is an extension to Hoedown
MIT License
23 stars 15 forks source link

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

Open jasharpe opened 9 years ago

jasharpe commented 9 years ago

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.

joeltine commented 9 years ago

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.

mildsunrise commented 9 years ago

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...

uranusjr commented 9 years ago

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.

mildsunrise commented 9 years ago

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.

gagern commented 4 years ago

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.