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

Some failures of markdown-it JS test fixtures #23

Open
chrisjsewell opened this issue Jun 22, 2023 · 6 comments
Open

Some failures of markdown-it JS test fixtures #23

chrisjsewell opened this issue Jun 22, 2023 · 6 comments

Comments

@chrisjsewell
Copy link
Member

chrisjsewell commented Jun 22, 2023

In https://github.com/chrisjsewell/markdown-it-pyrs/tree/main/tests/fixtures, I have essentially taken all the test fixtures that were present in markdown-it and applied them to this implementation.

I am currently skipping a few tests failing in commonmark_extras.md:

  • Don't output empty class here
  • Tabs should not cause hardbreak, EOL tabs aren't stripped in commonmark 0.27
  • Newline in image description

A few in normalize.md:

  • Keep %25 as is because decoding it may break urls, #720
  • Encode link destination, decode text inside it

and all of linkify.md, since the current plugin no longer parses "bare" URLs, e.g. it parses http://example.com but not www.example.com

Are these known issues/changes?

Note, ideally with this linkify, or perhaps in a separate plugin, it would obviously be desirable to create a GFM compliant rule for: https://github.github.com/gfm/#autolinks-extension-

@chrisjsewell
Copy link
Member Author

I guess, it would be great to have a mechanism in Rust to directly test these files.
There are crates like https://crates.io/crates/testing, https://crates.io/crates/rstest, and https://crates.io/crates/insta,
but I haven't found one that completely meets the criteria yet 🤔

@rlidwka
Copy link
Collaborator

rlidwka commented Jun 22, 2023

and all of linkify.md, since the current plugin no longer parses "bare" URLs, e.g. it parses http://example.com/ but not www.example.com

I use completely different linkify implementation, not related to JS in any way. Don't expect feature parity there.

You can open any issues you find at https://github.com/robinst/linkify

For bare urls, you can read this discussion: robinst/linkify#7

@rlidwka
Copy link
Collaborator

rlidwka commented Jun 22, 2023

The rest are probably bugs, which I'll have to look into shortly.

@rlidwka
Copy link
Collaborator

rlidwka commented Jun 25, 2023

The rest are probably bugs, which I'll have to look into shortly.

yep, that's three bugs out of the way: c2919dd, 1773eee, aa4bc65

@chrisjsewell
Copy link
Member Author

and all of linkify.md, since the current plugin no longer parses "bare" URLs, e.g. it parses http://example.com/ but not www.example.com

I use completely different linkify implementation, not related to JS in any way. Don't expect feature parity there.
You can open any issues you find at robinst/linkify

I'd note this not a limitation of linkify, which does parse these bare URLs:
it will parse emails like me@example.com by default, then requires finder.url_must_have_scheme(false) to parse e.g. www.example.com

The main limitation is in the plugin implementation here, which only searches for : https://github.com/rlidwka/markdown-it.rs/blob/c2919dd0b123f3aeb9264a6a6ec8a0d01cfbe19f/src/plugins/extra/linkify.rs#L80
so obviously will not identify any scheme-less URLs

I've now implemented https://crates.io/crates/markdown-it-autolink, for strict GFM autolink parsing.
The only known issue is that currently it cannot parse emails with _ before the @ e.g. a_b@example.com,
this is because it back-tracks from the @, but the _ has already been parsed as an emphasis token.
Not sure how yet to fix this 🤷‍♂️ (markdown-it-rust/markdown-it-plugins.rs#13)

@rlidwka
Copy link
Collaborator

rlidwka commented Jul 5, 2023

The main limitation is in the plugin implementation here, which only searches for :

ah okay, in original markdown-it we have two rules (inline rule that catches copy-pasted urls and core rule that catches everything), but here I've only implemented first one for simplicity

so the way to fix it is to re-scan all text later after inline parser is done, see:
https://github.com/markdown-it/markdown-it/blob/master/lib/rules_core/linkify.js

compare it to rule I've implemented here (and you probably copied):
https://github.com/markdown-it/markdown-it/blob/master/lib/rules_inline/linkify.js

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants