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.

Sign up for GitHub

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

Jump to bottom

Fix Issue #358 – preventing double nested links #433

Merged
merged 6 commits into from Feb 28, 2018
Merged

Fix Issue #358 – preventing double nested links #433

Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
d6d5f53
Fix Issue #358 – preventing double nested links
aidantwoods Oct 1, 2016
4d3600f
Extend disallowed assertion depth capabilities
aidantwoods Oct 2, 2016
50952b3
Line handler may prevent specified element nesting
aidantwoods Oct 2, 2016
a81aede
Line handler may prevent specified element nesting
aidantwoods Oct 4, 2016
543a6c4
Line handler may prevent specified element nesting
aidantwoods Oct 4, 2016
3aef89b
Line handler may prevent specified element nesting
aidantwoods Oct 8, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
34 changes: 5 additions & 29 deletions Parsedown.php
View file
Open in desktop
Expand Up @@ -1005,18 +1005,9 @@ public function line($text, $non_nestables=array())
{
# check to see if the current inline type is nestable in the current context

foreach ($non_nestables as $key => $non_nestable)
if (in_array($inlineType, $non_nestables))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about if (!empty($non_nestables) && in_array($inlineType, $non_nestables))? in_array() really is very expensive, even with empty arrays.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That sounds sensible, I'll commit that change when I get a sec :)

{
# case that we used array syntax
if (is_array($non_nestable) and $non_nestable[0] === $inlineType)
{
continue 2;
}
# case that we used plain string syntax
elseif ( ! is_array($non_nestable) and $non_nestable === $inlineType)
{
continue 2;
}
continue;
}

$Inline = $this->{'inline'.$inlineType}($Excerpt);
Expand All @@ -1040,26 +1031,11 @@ public function line($text, $non_nestables=array())
$Inline['position'] = $markerPosition;
}

# cause the new element to 'inherit' our non nestables, if appropriate
# cause the new element to 'inherit' our non nestables

foreach ($non_nestables as $key => $non_nestable)
foreach ($non_nestables as $non_nestable)
{
# array syntax, and depth is sufficient to pass on
if (is_array($non_nestable) and isset($non_nestable[1]) and
is_int($non_nestable[1]) and $non_nestable[1] > 1)
{
$Inline['element']['non_nestables'][] = array($non_nestable[0], $non_nestable[1] -1);
}
# array syntax, and depth is indefinite
elseif (is_array($non_nestable) and ! isset($non_nestable[1]))
{
$Inline['element']['non_nestables'][] = array($non_nestable[0]);
}
# string syntax, so depth is indefinite
elseif ( ! is_array($non_nestable))
{
$Inline['element']['non_nestables'][] = $non_nestable;
}
$Inline['element']['non_nestables'][] = $non_nestable;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about $Inline['element']['non_nestables'] = array_merge($Inline['element']['non_nestables'], $non_nestable); instead of the foreach (foreach is slightly slower btw, however, not to such an extent that it makes any difference)?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@PhrozenByte I did consider that, though there is no guarantee that the $Inline['element']['non_nestables'] will exist – so the code suffers a little for readability with that added check IMO. Though I appreciate that's a subjective decision, so I'm happy to favour array_merge if you disagree.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, didn't think about that. I agree that adding another check doesn't improve readability, so it's probably best to keep it the way you did 😃

}

# the text that comes before the inline
Expand Down