List item content wrapped in paragraphs

[ndreynolds]

I noticed an interesting discrepancy between Earmark and other markdown parsers I've tried. Probably related to #93, but it seems a bit different. Take the following Markdown example:

* Groceries
  * Produce
    * Apples
    * Carrots

Pandoc, CommonMark, Python Markdown, and redcarpet all produce the following HTML:

<ul>
  <li>
    Groceries
    <ul>
      <li>
        Produce
        <ul>
          <li>Apples</li>
          <li>Carrots</li>
        </ul>
      </li>
    </ul>
  </li>
</ul>

whereas Earmark produces this:

<ul>
  <li>
    <p>Groceries</p>
    <ul>
      <li>
        <p>Produce</p>
        <ul>
          <li>Apples</li>
          <li>Carrots</li>
        </ul>
      </li>
    </ul>
  </li>
</ul>

You can see that Earkmark adds paragraph tags whenever the list item contains a nested list. Using this comparison tool, I only see one other implementation (Gambas) with this behavior:

http://johnmacfarlane.net/babelmark2/?text=*+Groceries%0A++*+Produce%0A++++*+Apples%0A++++*+Carrots

I noticed it because it leads to inconsistent vertical spacing in the lists in my library's documentation (using ExDoc), since p elements have a 1em top and bottom margin by default in most browsers. Here's a visual example from my project:

Earmark via ExDoc	GitHub Markdown

In comparison to the GitHub version, the Earmark version looks very disconnected to me.

Is this something you'd be open to changing? If so, I'd be happy to try creating a PR that adjusts the output to match the other implementations.

[RobertDober]

Thank you for putting this to our attention.
I guess the <p>s around are indeed not the best way of rendering and I would be very glad to accept PRs in general.
However there might be a major issue with backwards compatibility, especially with ex_doc.
Maybe we should discuss it there firstly?

[RobertDober]

@ndreynolds on a personal note

Babelmark 2 has been a most valuable tool (and I had an Earmark dingus running for a couple of years)
now it seems that it is outdated and I usually use Babelmark III, which does confirm your findings too BTW 😁.
Sadly I do not have the time to setup a dingus right now, would be nice exposure of Elixir, but time is such a sparse resource 😢

[ndreynolds]

Sure, then I'll open a similar issue in ExDoc to start the discussion, and we can go from there.

Sadly I do not have the time to setup a dingus right now, would be nice exposure of Elixir, but time is such a sparse resource cry

I can set one up. I saw Gigalixir has a free tier, which seems like a good fit for this. It would certainly be helpful to have when testing this. Will give it a go later.

[RobertDober]

That would just be great.

…

-- Non datemi consigli che so sbagliare da solo (Giuseppe Di Stefano)

On Sun, Mar 31, 2019 at 11:13 PM Nick Reynolds ***@***.***> wrote: Sure, then I'll open a similar issue in ExDoc to start the discussion, and we can go from there. Sadly I do not have the time to setup a dingus right now, would be nice exposure of Elixir, but time is such a sparse resource cry I can set one up. I saw Gigalixir has a free tier, which seems like a good fit for this. It would certainly be helpful to have when testing this. Will give it a go later. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#226 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAEldwSw3G7ejErKCsR2eG16PLpBCHAGks5vcSUKgaJpZM4cUFIY> .

[RobertDober]

Backward incompatibility accepted here by @josevalim.
@ndreynolds shall I assign myself or do you want to prepare a PR. That will not be an easy one I am afraid.

[ndreynolds]

@RobertDober I'd be happy to take on the challenge if you don't mind. I have some time this week.

It seems like this is where I ought to start looking:

https://github.com/pragdave/earmark/blob/master/lib/earmark/html_renderer.ex#L129

I'd play with it some today and try to understand what would need to be changed.

[RobertDober]

@ndreynolds looks about the right spot. I appreciate how you handled this so far and do not mind at all if you take this on.

[ndreynolds]

I dug into this a bit more and found this in the CommonMark spec:

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.
(The difference in HTML output is that paragraphs in a loose list are wrapped in <p> tags, while paragraphs in a tight list are not.)

https://spec.commonmark.org/0.28/#loose

So it's a bit more nuanced than I thought, but this is pretty clear from an implementation standpoint. I think it means we need to identify whether the list is loose or tight and then apply (or not apply) the <p> tags accordingly. I'll keep working on it this week, hope to have a PR soon.

[RobertDober]

Great
Maybe we should start with adapting the acceptance tests to the behavior we want?
Shall I try to find some time to do that, or can you start your PR with it, so that we agree on it before you start implementation?

[RobertDober]

Please make your PR against https://github.com/pragdave/earmark/tree/226-loose-tight-lists
so that I can merge with the tests failing.

[ndreynolds]

@RobertDober done, I created #229 with failing tests, so we can discuss further there if you like.

[RobertDober]

Redundant to #249