[lexical-markdown] Bug Fix: Allow any characters in markdown link text#7735
[lexical-markdown] Bug Fix: Allow any characters in markdown link text#7735etrepum merged 4 commits intofacebook:mainfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
etrepum
left a comment
There was a problem hiding this comment.
In this sort of case it would be really nice to have some additional tests to show that this works as expected and also doesn't have edge cases, such as testing two independent links that are in the same line of text
|
Indeed, I will look into this |
1. Basic case with opening and closing brackets 2. Closing bracket with no corresponding opening one 3. Opening bracket with no corresponding closing one 4. Opening bracket with a bracket combination in title
etrepum
left a comment
There was a problem hiding this comment.
I think that maybe some revisions could be done here to more closely match how other commonmark parsers work, provided examples for the commonmark dingus and github
| html: '<p><a href="https://lexical.dev"><span style="white-space: pre-wrap;">hello]</span></a><a href="https://lexical.dev"><span style="white-space: pre-wrap;">world</span></a></p>', | ||
| md: '[hello]](https://lexical.dev)[world](https://lexical.dev)', |
There was a problem hiding this comment.
This one parses differently in commonmark, you can try it here https://spec.commonmark.org/dingus/
<p>[hello]](https://lexical.dev)<a href="https://lexical.dev">world</a></p>This is how github parses it:
[hello]](https://lexical.dev)[world](https://lexical.dev)
<p dir="auto">[hello]](<a href="https://lexical.dev)%5Bworld%5D(https://lexical.dev" rel="nofollow">https://lexical.dev)[world](https://lexical.dev</a>)</p>| html: '<p><a href="https://lexical.dev"><span style="white-space: pre-wrap;">hello[</span></a><a href="https://lexical.dev"><span style="white-space: pre-wrap;">world</span></a></p>', | ||
| md: '[hello[](https://lexical.dev)[world](https://lexical.dev)', |
There was a problem hiding this comment.
also this one
<p>[hello<a href="https://lexical.dev"></a><a href="https://lexical.dev">world</a></p>This is how github parses it:
[helloworld
<p dir="auto">[hello<a href="https://lexical.dev" rel="nofollow"></a><a href="https://lexical.dev" rel="nofollow">world</a></p>| html: '<p><a href="https://lexical.dev" title="ti[t]le"><span style="white-space: pre-wrap;">hello[</span></a><a href="https://lexical.dev" title="tit]le"><span style="white-space: pre-wrap;">world[</span></a></p>', | ||
| md: '[hello[](https://lexical.dev "ti[t]le")[world[](https://lexical.dev "tit]le")', |
There was a problem hiding this comment.
and this one
<p>[hello<a href="https://lexical.dev" title="ti[t]le"></a>[world<a href="https://lexical.dev" title="tit]le"></a></p>this is how github parses it:
<p dir="auto">[hello<a href="https://lexical.dev" title="ti[t]le" rel="nofollow"></a>[world<a href="https://lexical.dev" title="tit]le" rel="nofollow"></a></p>|
I totally understand the will to get close to those references. I'm also not omniscient in markdown so maybe I'm missing something but I believe the solution I'm suggesting allow to handle more edge cases and it seems to work quite well when looking at tests. |
…sponding closed one. This follows commonmark rules: - [h[ello](https://lexical.dev) -> ello will be linked - [h]ello](https://lexical.dev) -> no link
etrepum
left a comment
There was a problem hiding this comment.
This looks consistent with other Markdown implementations, nice work!
Description
In markdown view, type
[[Title] description ](https://example.com) [test [squared] abc](https://example.com)Switch off markdown to see how the link is formatted.
Before
Link text doesn't handle square brackets properly
Notice square bracket isn't part of the link for the first example and how it doesn't properly format the second one.
After
Link text can now accept any character in the text part
In this example the square bracket is now part of the link and everything is correctly formatted