Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Using micromark-extension-definition-list does not work if micromark-extension-gfm-table is used #8

Closed
4 tasks done
elbakerino opened this issue Sep 27, 2022 · 15 comments
Labels
👀 no/external This makes more sense somewhere else 👎 phase/no Post cannot or will not be acted on

Comments

@elbakerino
Copy link

elbakerino commented Sep 27, 2022

Initial checklist

Affected packages and versions

(latest) remark v14.0.2, remark-gfm v3.0.1, remark-definition-list v1.2.0, remark-rehype v10.1.0

Link to runnable example

https://stackblitz.com/edit/typescript-yj7hye?file=index.ts,index.html

Steps to reproduce

Using unified with remark-gfm and remark-definition-list, compiling with rehype.

It does not matter what is used first: remarkGfm or remarkDefinitionList.

The output of remark looks correct, but not the HTML when using unified.

The example contains one gfm with gfm-table plugin (1:1 copy from repo) and one without it, when gfm-table is not used, it works correct.


as not possible to select:

Package manager: NPM v8

Build and bundle tools: none, plain ESM

Expected behavior

No impact of micromark-extension-gfm-table when using the definition list syntax:

First Term
: This is the definition of the first term.

Second Term
: This is one definition of the second term.
: This is another definition of the second term.

Like GitHub, e.g. just not handling it:

First Term
: This is the definition of the first term.

Second Term
: This is one definition of the second term.
: This is another definition of the second term.


This HTML is expected:

<dl>
<dt>First Term</dt>
<dd>This is the definition of the first term.
</dd>
<dt>Second Term</dt>
<dd>This is one definition of the second term.
</dd>
<dd>This is another definition of the second term.
</dd>
</dl>

Actual behavior

When using micromark-extension-gfm-table it forces, without that plugin it defaults to be compiled to HTML:

<p>First Term
: This is the definition of the first term.</p>
<p>Second Term
: This is one definition of the second term.
: This is another definition of the second term.</p>

Whereas GitHub creates:

<p>First Term<br>
: This is the definition of the first term.</p>
<p>Second Term<br>
: This is one definition of the second term.<br>
: This is another definition of the second term.</p>

Runtime

Node v14

Package manager

Other (please specify in steps to reproduce)

OS

Windows, Linux

Build and bundle tools

Other (please specify in steps to reproduce)

@github-actions github-actions bot added 👋 phase/new Post is being triaged automatically 🤞 phase/open Post is being triaged manually and removed 👋 phase/new Post is being triaged automatically labels Sep 27, 2022
@wooorm
Copy link
Member

wooorm commented Sep 27, 2022

What does this have to do with this extension?

@wooorm
Copy link
Member

wooorm commented Sep 27, 2022

Press the “Update all to latest” (refresh wheel) above the dependencies.
That makes it work for me.

@elbakerino
Copy link
Author

When not using this extension it works as expected, after quite a bit of debugging none other changes than disabling this plugin worked.

Thus the three examples in the stackblitz.

@elbakerino
Copy link
Author

Updated dependencies

@wooorm
Copy link
Member

wooorm commented Sep 27, 2022

OK I found your request rather confusing, but you’re right that there is a problem. Here’s a reduced case:

import { micromark } from "micromark";
import { gfmTable, gfmTableHtml } from "micromark-extension-gfm-table";
import { defList, defListHtml } from "micromark-extension-definition-list";

const markdown = `a
: b

| c |
| - |
`;

const output = micromark(markdown, {
  extensions: [defList, gfmTable],
  htmlExtensions: [defListHtml, gfmTableHtml],
});

console.log(output);

Passing gfmTable (or not) changes the definition list.
Or, the other way around: using defList, does not work if gfmTable is passed.

Have you though about treating this issue the other way around: what if this is something in defList?

@wooorm wooorm changed the title Handling/Changing definition-list syntax somewhere Using micromark-extension-definition-list does not work if micromark-extension-gfm-table is used Sep 27, 2022
@elbakerino
Copy link
Author

elbakerino commented Sep 27, 2022

First wanted to open it in gfm - but then dug deeper and found the relation to this extension here. Can only describe it as easy as it is to understand what is happening where. I'm a new developer of micromark plugins thus the internals are (still?!) a bit complex to grasp after a few hours migrating from markdown-it.

Yes - there maybe something missing in defList.

See the "expected behaviour", GitHub is handling table-detection (or parsing or whatever) otherwise than the plugin - (or may have something completely else for that).

Thus I'm assuming, when making this gfm-table plugin to behave like GitHub tables, it would fix it also, when this doesn't fix it of course than the other extension need to be adjusted as well.

Otherwise - i'm still searching for the part in the gfm spec. - shouldn't the general gfm plugin include something to behave like GitHub?

i've changed the "expected behaviour" to:

  • include the GitHub HTML
  • clearify that the gfm-table plugin forces the (faulty) HTML
  • clearify that without the gfm-table plugin it defaults to the (faulty) HTML

@elbakerino
Copy link
Author

Didn't found anything in GFM spec related to ": as line feed" - the different <br/> comes more from the general "soft break to <br/>" handling of GitHub.

Thus my current guess is that when the gfm-table plugin doesn't "force" a paragraph, the defList plugin will just work fine.

If there isn't something like "forcing nodes" at all available/implemented, I'll of course move this issue to the defList plugin.

@wooorm
Copy link
Member

wooorm commented Sep 27, 2022

GFM spec

GFM does not support definition lists

forcing nodes

This extension does not “force” paragraphs.


gfm-table plugin to behave like GitHub tables

This extension behaves like GitHub. If not, that’s a bug. Can you prove a case, without definition lists, where that isn’t the case?

@wooorm
Copy link
Member

wooorm commented Oct 7, 2022

Friendly ping! :)

@elbakerino
Copy link
Author

Sorry needed a while to get back to it.

Thanks for your confirmations, so I didn't need to check in that direction.

After further debugging I've found that the issue seems to be with some reformatting of blanks, when the gfmTable module is used.

When using gfmTable + defList the spaces are changed.

For example pandoc/markdown extra php requires three spaces after the colon and before the content.

The defList plugin changes 1 space to 3 spaces, but with gfm this is changed back again.

I don't know if the gfmTable plugin shouldn't do something it does with spaces, or that defList shouldn't rely/change those like currently done.

But even when using 3 spaces in MarkDown source, together with the gfmTable plugin the lists don't work: stackblitz demo.

Example

I've created a stackblitz demo shows all variants.

Initial Spaces: 1

md_1 uses gfm + defList, the blanks stay at 1

# Definition List

First Term
: This is the definition of the first term.

Second Term
: This is one definition of the second term.
: This is another definition of the second term.

md_2 uses defList, the blanks go to 3

# Definition List

First Term
:   This is the definition of the first term.

Second Term
:   This is one definition of the second term.
:   This is another definition of the second term.

md_3 uses gfm, the blanks stay at 1

# Definition List

First Term
: This is the definition of the first term.

Second Term
: This is one definition of the second term.
: This is another definition of the second term.

Initial Spaces: 3

md_4 uses gfm + defList, the blanks are already at 3, they stay at 3

# Definition List

First Term
:   This is the definition of the first term.

Second Term
:   This is one definition of the second term.
:   This is another definition of the second term.

md_5 uses gfm, the blanks are already at 3, they stay at 3

# Definition List

First Term
:   This is the definition of the first term.

Second Term
:   This is one definition of the second term.
:   This is another definition of the second term.

md_6 uses defList, the blanks are already at 3, they stay at 3

# Definition List

First Term
:   This is the definition of the first term.

Second Term
:   This is one definition of the second term.
:   This is another definition of the second term.

@wooorm
Copy link
Member

wooorm commented Oct 17, 2022

with some reformatting of blanks

I think you’re observing something, but your conclusion is incorrect.
The conclusion I believe is:

  • If everything works, definition lists are indented as 3 spaces (md_2, md_4, md_6)
  • If it doesn’t work, indentation stays as-is (md_1, md_3, md_5)

But see my reproduction above: #8 (comment). This isn’t about reformatting. It’s about parsing correctly or not. GFM + deflist doesn’t work. In HTML that means that no <dl>s are generated. In markdown it means that indentation isn’t generated.

Reiterating my earlier question:

Have you though about treating this issue the other way around: what if this is something in defList?

Have you tried asking there?

@binyamin
Copy link

binyamin commented Jan 5, 2023

@elbakerino Does it work using micromark-extension-definition-list@v1.3.0?
See wataru-chocola/micromark-extension-definition-list#72 (comment)

@elbakerino
Copy link
Author

Yes the upgrade fixed it and defLists are now working together with gfm!

@github-actions

This comment has been minimized.

@wooorm wooorm added the 👀 no/external This makes more sense somewhere else label Feb 10, 2023
@github-actions

This comment was marked as resolved.

@github-actions github-actions bot added 👎 phase/no Post cannot or will not be acted on and removed 🤞 phase/open Post is being triaged manually labels Feb 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
👀 no/external This makes more sense somewhere else 👎 phase/no Post cannot or will not be acted on
Development

No branches or pull requests

3 participants