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

Fix handling of reference links with inline code #24

Closed
wants to merge 1 commit into from

Conversation

Trott
Copy link
Contributor

@Trott Trott commented Sep 19, 2021

Initial checklist

  • I read the support docs
  • I read the contributing guide
  • I agree to follow the code of conduct
  • I searched issues and couldn’t find anything (or linked relevant results below)
  • If applicable, I’ve added docs and tests

Description of changes

This fixes the AST so that certain kinds of reference links are not
broken when converted back to markdown.

Fixes: remarkjs/remark#850

This fixes the AST so that certain kinds of reference links are not
broken when converted back to markdown.

Fixes: remarkjs/remark#850
@github-actions github-actions bot added the 👋 phase/new Post is being triaged automatically label Sep 19, 2021
@github-actions

This comment has been minimized.

@github-actions github-actions bot added 🤞 phase/open Post is being triaged manually and removed 👋 phase/new Post is being triaged automatically labels Sep 19, 2021
@Trott
Copy link
Contributor Author

Trott commented Sep 19, 2021

I don't know if this is the right fix for remarkjs/remark#850, but it fixes that issue without breaking any existing tests, so I'm hopeful....

@ChristianMurphy ChristianMurphy added 🕸️ area/tests This affects tests 🗄 area/interface This affects the public interface labels Sep 19, 2021
@wooorm
Copy link
Member

wooorm commented Sep 19, 2021

Nice catch! It looks like definitions do get a proper identifier and label, but for link (and image?) references it’s broken.

There is probably a problem with character references and character escapes though. Take this example:

[`f`][]
[;][]
[\;][]
[;][]
[`f`;][]
[`f`\;][]
[`f`;][]

[`f`]: alpha
[;]: bravo
[\;]: charlie
[;]: delta
[`f`;]: echo
[`f`\;]: foxtrot
[`f`;]: golf

Note that one might expect that character references and -escapes and the actual value would match, but CommonMark does not:

<p><a href="alpha"><code>f</code></a>
<a href="bravo">;</a>
<a href="charlie">;</a>
<a href="delta">;</a>
<a href="echo"><code>f</code>;</a>
<a href="foxtrot"><code>f</code>;</a>
<a href="golf"><code>f</code>;</a></p>

Looking at the AST that remark produces, the definitions work properly: the character references and -escapes are handled properly in label/identifier. The same is true for link (and image?) references, but there the code (and probably other things such as emphasis), as you found out, don’t work.

@wooorm wooorm added the 🙆 yes/confirmed This is confirmed and ready to be worked on label Sep 19, 2021
@github-actions github-actions bot added 👍 phase/yes Post is accepted and can be worked on and removed 🤞 phase/open Post is being triaged manually labels Sep 19, 2021
@github-actions

This comment has been minimized.

@wooorm
Copy link
Member

wooorm commented Sep 19, 2021

I was able to verify that micromark does handle this correctly: micromark/micromark@5be7549

wooorm added a commit to micromark/micromark that referenced this pull request Sep 19, 2021
Add a new utility, `micromark-util-decode-string`, which helps decode
character escapes and -references as found in the string content type
of markdown.
This is not used in `micromark` itself but is useful for extensions and
mdast utilities.

Related to syntax-tree/mdast-util-from-markdown#24.
Related to remarkjs/remark#850.
wooorm added a commit that referenced this pull request Sep 19, 2021
@wooorm wooorm closed this in a90b930 Sep 19, 2021
@github-actions

This comment has been minimized.

@wooorm wooorm added the 💪 phase/solved Post is done label Sep 19, 2021
@github-actions github-actions bot removed 👍 phase/yes Post is accepted and can be worked on 🙆 yes/confirmed This is confirmed and ready to be worked on labels Sep 19, 2021
@wooorm
Copy link
Member

wooorm commented Sep 19, 2021

Thanks @Trott, this helped a lot. Released!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🗄 area/interface This affects the public interface 🕸️ area/tests This affects tests 💪 phase/solved Post is done
Development

Successfully merging this pull request may close these issues.

stringify() appears to mishandle reference links with inline code
3 participants