syntax-tree / mdast-util-to-hast

utility to transform mdast to hast
https://unifiedjs.com
MIT License
104 stars 43 forks source link

Allow disabling loose lists #73

Closed stevemk14ebr closed 8 months ago

stevemk14ebr commented 8 months ago

Initial checklist

Problem

With nested lists, the loose flag in listItem causes newlines at the start of line items that breaks proper list rendering. See

<ul>
<li><div style="border: 1px solid rgba(255, 255, 255, 0.15); display: inline-block; border-radius: 4px; width: fit-content;"><span class="markdown" style="list-style-position: outside; display: inline; font-family: monospace; text-decoration: none; white-space: pre-wrap;"><code>hxxps[:]//evil[.]com/</code></span></div>
<div style="border: 1px solid rgba(255, 255, 255, 0.15); display: inline-block; border-radius: 4px; width: fit-content;"><span class="markdown" style="list-style-position: outside; display: inline; font-family: monospace; text-decoration: none; white-space: pre-wrap;"><ul>
<li><code>uri</code>: <code>/update.bin</code>
<ul>
<li><code>domainname</code>: <code>evil[.]com</code></li>
<li><code>scheme</code>: <code>hxxps</code></li>
</ul>
</li>
</ul></span></div></li>
</ul>

vs loose list:

<ul>
<li>
<p><div style="border: 1px solid rgba(255, 255, 255, 0.15); display: inline-block; border-radius: 4px; width: fit-content;"><span class="markdown" style="list-style-position: outside; display: inline; font-family: monospace; text-decoration: none; white-space: pre-wrap;"><code>hxxps[:]//evil[.]com/</code></span></div></p>
<p><div style="border: 1px solid rgba(255, 255, 255, 0.15); display: inline-block; border-radius: 4px; width: fit-content;"><span class="markdown" style="list-style-position: outside; display: inline; font-family: monospace; text-decoration: none; white-space: pre-wrap;"><ul>
<li><code>uri</code>: <code>/update.bin</code>
<ul>
<li><code>domainname</code>: <code>evil[.]com</code></li>
<li><code>scheme</code>: <code>hxxps</code></li>
</ul>
</li>
</ul></span></div></p>
</li>
</ul>

The issue is specifically:

<ul>
<li>
<p>

The start of the

block is put on a newline after the li and this newline causes the bullet to be on the wrong line. In the first case it goes:

<ul>
<li><div...

Where the div is immediately after with no newline and this renders correctly.

image

The markdown here is:

## Connections

*   `hxxps[:]//evil[.]com/`

    *   `uri`: `/update.bin`
        *   `domainname`: `evil[.]com`
        *   `scheme`: `hxxps`

Compare this to a markdown without a space between the first bullet and the next ones. (Not a loose list). image With the markdown being:

## Connections

*   `hxxps[:]//evil[.]com/`\
    *   `uri`: `/update.bin`
        *   `domainname`: `evil[.]com`
        *   `scheme`: `hxxps`

So you can see how this is undesireable, the bullet should stay aligned to the correct row but it's not when loose lists are detected. Please allow an option to disable loose lists, I cannot have this newline character present at the start of the listItem.

Solution

Add an optional flag to disable loose lists (https://github.com/syntax-tree/mdast-util-to-hast/blob/c7a658efaf31316fe01033bada3e5f396cbc6576/lib/handlers/list-item.js#L66-L74)

Alternatives

Change listItem to not place a newline before the listItem.

stevemk14ebr commented 8 months ago
import { visit } from 'unist-util-visit';
const splice = [].splice

export default function rehypeFixList(options) {
  return transform;

  function transform(tree) {
    visit(tree, 'element', onelement)
  }

  function onelement(node, index, parent) {
    if(node.tagName == 'li' && node.children.length > 1 && node.children[0].type == 'text' && node.children[0].value == '\n') {
        node.children = node.children.slice(1); //remove first newline after an li
    }
    return undefined;
  }
}

This is my custom plugin I put in the pipeline to fix this for now

wooorm commented 8 months ago

What do you want to happen if there are two paragraphs next to each other?

Why is whitespace problematic? You can use CSS to change it?

Why should there be an option to do this instead of a plugin? Most things happen with plugins. Especially if not all users would want something, in a particular way.

stevemk14ebr commented 8 months ago

There is no CSS layout I could find that would fix the newline before the li and the paragraph tag. If you set display: inline it will override correct bullet layouts in other situations.

As my image shows, this newline causes the bullet to render on the wrong line. The issue is not two paragraphs next to each other, the issue is that when this happens the first paragaph is put on a newline after the li opens:

this is wrong:

<ul>
<li>
<p>

it should be this

<ul>
<li><p>

the non-loose list generates this correct, but starts with the content directly and doesn't wrap it in a paragraph, it emits:

<ul>
<li><div>

New lines inbetween the paragraphs would be expected and correct. But there are newlines inserted before the first paragraph when loose list is detected.

stevemk14ebr commented 8 months ago

Suggested fix might be something like this: https://github.com/syntax-tree/mdast-util-to-hast/blob/c7a658efaf31316fe01033bada3e5f396cbc6576/lib/handlers/list-item.js#L63C2-L82

 let firstChildAdded = false;
  while (++index < results.length) {
    const child = results[index]

    // Add eols before nodes, except if this is a loose, first paragraph.
    if (firstChildAdded && (
      loose ||
      index !== 0 ||
      child.type !== 'element' ||
      child.tagName !== 'p')
    ) {
      children.push({type: 'text', value: '\n'})
    }

    if (child.type === 'element' && child.tagName === 'p' && !loose) {
      children.push(...child.children)
    } else {
      children.push(child)
    }
    firstChildAdded = true;
  }

You could also change the order of the operations to add the newlines after the first child is added instead. But you'd have to detect the final child and not emit a newline in that case.

ChristianMurphy commented 8 months ago

this is wrong:

<ul>
<li>
<p>

it should be this

<ul>
<li><p>

Looking at CommonMark https://spec.commonmark.org/dingus/?text=%23%23%20Connections%0A%0A*%20%20%20%60hxxps%5B%3A%5D%2F%2Fevil%5B.%5Dcom%2F%60%0A%0A%20%20%20%20*%20%20%20%60uri%60%3A%20%60%2Fupdate.bin%60%0A%20%20%20%20%20%20%20%20*%20%20%20%60domainname%60%3A%20%60evil%5B.%5Dcom%60%0A%20%20%20%20%20%20%20%20*%20%20%20%60scheme%60%3A%20%60hxxps%60 The reverse is true, according to the reference implementation. The first output is correct, it should not be the second.

As @wooorm mentions you are welcome to use CSS to adjust the styles, or create a plugin to adjust the content if you prefer that over CSS https://unifiedjs.com/learn/

github-actions[bot] commented 8 months ago

Hi! This was closed. Team: If this was fixed, please add phase/solved. Otherwise, please add one of the no/* labels.

stevemk14ebr commented 8 months ago

Ah ok thanks, I see what's happening.

Copying the styles used in the reference page works to fix the loose list bullets. However, in my case those markdown directives you see are rendered as spans containing content and marked as display: inline-block which breaks the layout in normal lists.

So for me, this is a bad interaction between my custom code, the newline rehype adds, and typical css bullet layouts. I'll stick with my custom pass to remove the newline since this is unique to my implementation constraints

wooorm commented 8 months ago

Yes, I was about to say but you seem to reach similar conclusions: this is regular markdown/html behavior. I haven't heard of your problem before. So there must be something specific to your case. Directives, in this case an inline block one, seems to be the source. Not sure how to handle it, perhaps you can use a different directive kind, or a different display?

stevemk14ebr commented 8 months ago

I have it working now, I need my directives to be rendered as display: inline because users may choose to write content with words on either side with the directive in the middle holding some static markdown content. But in other cases (like when they hold bullets) the directives should instead be display: block. A good solution would be me changing my code to change the display style depending on the content but in reality this issue only occurs with this one specific case of loose lists so I'm choosing to just edit the rehype AST myself and remove the troublesome newline. The newline interacts poorly with my display inline and whitespace css.

All is well, sorry for the noise, I can work around this easily with my custom plugin pass.