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

Code blocks with email-like text are no longer parsed as code #1218

Closed
kornelski opened this issue Apr 15, 2018 · 13 comments · Fixed by #1337
Closed

Code blocks with email-like text are no longer parsed as code #1218

kornelski opened this issue Apr 15, 2018 · 13 comments · Fixed by #1337
Labels
help wanted L1 - broken Valid usage causes incorrect output OR a crash AND there is no known workaround for the issue

Comments

@kornelski
Copy link

kornelski commented Apr 15, 2018

Marked version: 0.3.12 (OK) 0.3.14 (buggy)

`user@host`

used to render as <code> block:

user@host

but now it's rendered as an e-mail link with literal ` in it:

`user@host`

kornelski added a commit to ladjs/superagent that referenced this issue Apr 15, 2018
@UziTech
Copy link
Member

UziTech commented Apr 15, 2018

seems to work in the latest 0.3.19: demo

@kornelski
Copy link
Author

kornelski commented Apr 15, 2018

Sorry, I've oversimplified the example. Try:


 `/users?email=joe@smith.com`.

@UziTech
Copy link
Member

UziTech commented Apr 15, 2018

Yup `user@host.com` does it

@UziTech UziTech added the L1 - broken Valid usage causes incorrect output OR a crash AND there is no known workaround for the issue label Apr 15, 2018
@joshbruce
Copy link
Member

joshbruce commented Apr 15, 2018

Looking at the specs to try and figure out a test to create and they don't seem to call out this specific use case.

For CommonMark:

Autolinks are absolute URIs and email addresses inside < and >.

So, this would be something related to GitHub Flavored Markdown:

An extended email autolink will be recognised when an email address is recognised within any text node.

Looking at code spans (examples 322 to 327) I would suspect that the proper behavior would be something like:

Markdown:

`user@example.com`

Html:

<p><code>user@example.com</code></p>

@kornelski: Is that what you're looking for?

@kornelski
Copy link
Author

kornelski commented Apr 15, 2018

In my case <code> is more important than <a>, because I'm using a made-up e-mail address in documentation and don't want it linked. If you prefer links, then both tags together (<code><a>email</a></code>) would be a reasonable rendering IMHO.

@UziTech
Copy link
Member

UziTech commented Apr 15, 2018

IMO creating a link without <> surrounding it should only happen when the gfm option is true since this would not be compliant with commonmark

@joshbruce
Copy link
Member

GFM complies with CommonMark; so, autolinking should be invoked by the <> combination. Given what I can discern from the code span area of CM and GFM, I would say the code span leaves everything inside alone. The linked PR runs a test that would result in that ignore.

@UziTech
Copy link
Member

UziTech commented Apr 16, 2018

in order to pass CommonMark Example #583 This autolinking without <> will have to be moved to gfm: true

@joshbruce
Copy link
Member

Think this conversation might go beyond this issue...deeper issues with Marked at the moment...making a new issue:

@joshbruce
Copy link
Member

To pass I think we actually need a way to specify just CommonMark - not GFM or Pedantic. We currently don't have anything like that. The CommonMark Autolinks remain in the CommonMark spec tests - where they are at present - of which 583 is already listed as failing but should pass.

@Feder1co5oave
Copy link
Contributor

According to gfm spec, autolinks have higher precedence than code spans.

Also, email addresses are recognized and autolinked by the InlineLexer.rules.gfm.url rule, which (re-)uses the InlineLexer.rules._email subrule. This comes from cm spec. This is wrong, since gfm defines extended autolinked email adresses quite differently.

I'm making a PR for this

@UziTech
Copy link
Member

UziTech commented Sep 18, 2018

this is fixed by #1337

@Feder1co5oave
Copy link
Contributor

Aw, that PR was not linked :(

I have done a totally different solution, I'm not sure what you were going for with that PR. I'll just submit it and then you'll decide what to pick and/or integrate one with the other!

Feder1co5oave added a commit to Feder1co5oave/marktex that referenced this issue Sep 18, 2018
@UziTech UziTech mentioned this issue Sep 18, 2018
4 tasks
zhenalexfan pushed a commit to zhenalexfan/MarkdownHan that referenced this issue Nov 8, 2021
zhenalexfan pushed a commit to zhenalexfan/MarkdownHan that referenced this issue Nov 8, 2021
zhenalexfan pushed a commit to zhenalexfan/MarkdownHan that referenced this issue Nov 8, 2021
Morita0711 added a commit to Morita0711/npm-superurgent that referenced this issue Jul 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted L1 - broken Valid usage causes incorrect output OR a crash AND there is no known workaround for the issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants