Skip to content
This repository was archived by the owner on Apr 4, 2020. It is now read-only.

links with underscores linked incorrectly #10

Closed
glensc opened this issue Mar 27, 2019 · 4 comments
Closed

links with underscores linked incorrectly #10

glensc opened this issue Mar 27, 2019 · 4 comments

Comments

@glensc
Copy link
Contributor

glensc commented Mar 27, 2019

smartpunct and autolink don't play well

input:

it’s
https://eventum.example.net/history.php?iss_id=107092
https://gitlab.example.net/group/project/merge_requests/39#note_150630

output:

<p>it’s<br />
<a href="https://eventum.example.net/history.php?iss">https://eventum.example.net/history.php?iss</a>_id=107092<br />
<a href="https://gitlab.example.net/group/project/merge">https://gitlab.example.net/group/project/merge</a>_requests/39#note_150630</p>

remove the apostrophe and links get formatted properly:

<p>it<br />
<a href="https://eventum.example.net/history.php?iss_id=107092">https://eventum.example.net/history.php?iss_id=107092</a><br />
<a href="https://gitlab.example.net/group/project/merge">https://gitlab.example.net/group/project/merge</a>_requests/39#note_150630</p>

wait, the second link still wrong.

doesn't matter which apostrophe being used, or are they in pairs:

  • it's, it's ' ...

versions (if that's relevant):

league/commonmark                  0.18.2               PHP Markdown parser...
league/commonmark-ext-autolink     v0.2.0               Extension for leagu...
league/commonmark-ext-inlines-only v0.1.0               Extension for leagu...
league/commonmark-ext-smartpunct   v0.1.0               Intelligently conve...
league/commonmark-extras           0.2.1                Useful extensions f...
@glensc
Copy link
Contributor Author

glensc commented Mar 27, 2019

maybe it's not smartpunct related, as this is also wrongly rendered:

https://eventum.example.net/history.php?iss_id=107092
https://gitlab.example.net/group/project/merge_requests/39#note_150630

while link alone is correct:

https://gitlab.example.net/group/project/merge_requests/39#note_150630

@glensc
Copy link
Contributor Author

glensc commented Mar 27, 2019

indeed. disabled smartpunct and problems persist

@glensc glensc changed the title smartpunct and autolink don't play well links with underscores linked incorrectly Mar 27, 2019
colinodell added a commit to thephpleague/commonmark that referenced this issue Mar 28, 2019
colinodell added a commit to thephpleague/commonmark that referenced this issue Mar 28, 2019
colinodell added a commit to thephpleague/commonmark that referenced this issue Mar 28, 2019
@colinodell
Copy link
Member

This is a bug with league/commonmark. In order to support autolinking, it needed to find consecutive Text elements and "collapse" them into one. This would clean up any leftover _ elements that failed to be parsed as emphasis, thus ensuring the entire link fit within a single Text element instead of being spread across several. We introduced this collapsing feature in 0.18.2 but it contained a bug where it wouldn't collapse all of them, only the first few.

Basically, it would take an AST similar to this (from the new unit test I'm adding):

$paragraph = new Paragraph();

$paragraph->appendChild(new Text('https://eventum.example.net/history.php?iss'));
$paragraph->appendChild(new Text('_'));
$paragraph->appendChild(new Text('id=107092'));
$paragraph->appendChild(new Newline(Newline::SOFTBREAK));
$paragraph->appendChild(new Text('https://gitlab.example.net/group/project/merge'));
$paragraph->appendChild(new Text('_'));
$paragraph->appendChild(new Text('requests/39#note'));
$paragraph->appendChild(new Text('_'));
$paragraph->appendChild(new Text('150630'));

And should have collapsed the adjoining Text elements to be equivalent to this:

$paragraph->appendChild(new Text('https://eventum.example.net/history.php?iss_id=107092'));
$paragraph->appendChild(new Newline(Newline::SOFTBREAK));
$paragraph->appendChild(new Text('https://gitlab.example.net/group/project/merge_requests/39#note_150630'));

But due to the implementation bug, it actually did something closer to this:

$paragraph->appendChild(new Text('https://eventum.example.net/history.php?iss_id=107092'));
$paragraph->appendChild(new Newline(Newline::SOFTBREAK));
$paragraph->appendChild(new Text('https://gitlab.example.net/group/project/merge'));
$paragraph->appendChild(new Text('_'));
$paragraph->appendChild(new Text('requests/39#note'));
$paragraph->appendChild(new Text('_'));
$paragraph->appendChild(new Text('150630'));

See how that second link is still split across multiple Text elements?

I'm preparing a fix and release for this now :)

@glensc
Copy link
Contributor Author

glensc commented Mar 28, 2019

thanks. super!

# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants