Fix Issue #358 – preventing double nested links #433

aidantwoods · 2016-10-01T14:56:23Z

Add the ability for a parsed element to enforce that the line handler not parse any (immediate) child elements of a specified type.
Use 1. to allow parsed Link elements to tell the line handler not to parse any child Links or Urls where they are immediate children.

1. Add the ability for a parsed element to enforce that the line handler not parse any (immediate) child elements of a specified type. 2. Use 1. to allow parsed Url elements to tell the line handler not to parse any child Links or Urls where they are immediate children.

aidantwoods · 2016-10-01T17:48:09Z

If we like 1. as a feature, then I can probably extend the functionality such that (disallowed) nesting can be asserted past a depth of one.

i.e. So although this PR fixes the original issue, cases such as:

[http://example.com](http://example.com)

~~It won't fix cases such as:~~
The following case is now also addressed in the second patch:

[*http://example.com*](http://example.com)

~~without being able to assert further than an immediate depth.~~

This is because the link can only currently assert nested exclusions for direct children. In the second example, the direct child of the Link is an italic element, and the inline Url is a child of the italic element (child of depth 2 relative to the Link).

I've built on the functionality of feature 1. in the previous commit to allow non nestables to be asserted indefinitely, or to a specified depth.

aidantwoods · 2016-10-02T16:44:43Z

I've built a little on the functionality as hinted in the above comment.

Inside the element array, one can now specify:
'non_nestables' => array('Url', 'Link'),
or
'non_nestables' => array(['Url'], ['Link']),
or mixed:
'non_nestables' => array('Url', ['Link']),
or the identical:
'non_nestables' => array('Url', array('Link')),

If you wish to only assert non nesting depth a finite number of times simply use the array syntax, and specify the depth assertion as the second array value.

e.g. the following will assert Link or Url may not be nested within a Link, at any depth, but Code may be inserted if greater than depth two:

So for the following data:

We achieve the following output:

Where inline Url is not ever allowed, but code is allowed past a depth of two

This commit serves to add comments detailing parts of the new functionality, and to adjust syntax preferences to match that of the surrounding document. The commit title also now reflects the most significant change made.

aidantwoods · 2016-10-04T12:00:16Z

@PhrozenByte @erusev any thoughts on the latest changes in this PR?

Just noticed another duplicate issue (#432) report popped up for this right before my PR, so looks like this fix is a much needed update

PhrozenByte · 2016-10-04T12:17:47Z

All in all: Looks great to me! 👍

However, I don't see a use case for specifying a maximum depth for non nestable types. I suggest removing the "array syntax" (i.e. 'non_nestables' => array('Url', 'Link') is the only supported syntax) and assuming that non nestable types are in effect for an infinite depth.

Removed granularity controls – elements are assumed to be non nestable indefinitely once declared.

aidantwoods · 2016-10-04T14:33:38Z

@PhrozenByte That makes sense to me!
Yeah, I couldn't immediately think of any useful cases for max nesting either – was an easy add on with the inheritance change though.

I've removed it for now, but if it's ever useful then it's easy to re-add if it's ever needed (and forward compatible with the current string syntax etc..)

It does simplify the code considerably not having the max depth and mixed syntax ;-)

PhrozenByte · 2016-10-04T14:47:25Z

Parsedown.php

-                    {
-                        $Inline['element']['non_nestables'][] = $non_nestable;
-                    }
+                    $Inline['element']['non_nestables'][] = $non_nestable;


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)?

@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.

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 😃

PhrozenByte · 2016-10-04T14:58:16Z

Parsedown.php

@@ -1003,6 +1003,13 @@ public function line($text)

            foreach ($this->InlineTypes[$marker] as $inlineType)
            {
+                # check to see if the current inline type is nestable in the current context
+
+                if (in_array($inlineType, $non_nestables))


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

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

Check if array is empty to shave some performance hits in the case than no non nestables are present.

aidantwoods · 2016-10-04T21:11:36Z

@PhrozenByte I think that's everything in the latest commit?

PhrozenByte · 2016-10-04T21:13:56Z

Yes, perfect 👍

aidantwoods · 2016-10-08T13:23:01Z

@erusev guessing as before, you'll want camel casing instead of '_' as the word delimiter?
Anything else that needs adjusting?

erusev · 2016-10-08T16:35:41Z

@aidantwoods

Yes, pls.

->line('abc *abc*', 'em') would produce abc *abc* and not parse the emphasis, right?

aidantwoods · 2016-10-08T16:50:41Z

@erusev almost – it would be ->line('abc *abc*', array('em')) or ->line('abc *abc*', ['em']), which would produce abc *abc* with no emphasis.

This is because it's built to allow multiple types to be disallowed, (including where multiple non nestables are aggregated over nested elements with different non nestable conditions). So line does have to be able to accept an array for that to function.

Swap `under_scores` for `camelCasing`

erusev · 2016-10-08T16:58:43Z

@aidantwoods

Right, well, then there might be a better name for the parameter.

Non nestables seems to scary of a word for a parameter that just lists the types that the method should skip.

aidantwoods · 2016-10-08T17:01:14Z

@erusev perhaps, any suggestions? 'skipInlineTypes', 'ignoredInlineTypes'?

erusev · 2016-10-08T17:05:29Z

Something like this.

Let me have a better look at the code later next week and I might come up with some ideas.

hatrena · 2017-05-18T14:53:41Z

Any chance to merge this PR ?

erusev · 2017-05-18T20:58:11Z

@hatrena Sure, I'll try to review it by the end of the month.

aidantwoods mentioned this pull request Oct 1, 2016

Parse inlineLink incorrectly if the link text is an URL #358

Closed

Extend disallowed assertion depth capabilities

4d3600f

I've built on the functionality of feature 1. in the previous commit to allow non nestables to be asserted indefinitely, or to a specified depth.

Line handler may prevent specified element nesting

50952b3

This commit serves to add comments detailing parts of the new functionality, and to adjust syntax preferences to match that of the surrounding document. The commit title also now reflects the most significant change made.

Line handler may prevent specified element nesting

a81aede

Removed granularity controls – elements are assumed to be non nestable indefinitely once declared.

PhrozenByte reviewed Oct 4, 2016

View reviewed changes

Line handler may prevent specified element nesting

543a6c4

Check if array is empty to shave some performance hits in the case than no non nestables are present.

Line handler may prevent specified element nesting

3aef89b

Swap `under_scores` for `camelCasing`

aidantwoods mentioned this pull request Mar 23, 2017

Bug with inlineUrl method - not consider already wrapped anchor with the same URL #487

Closed

This was referenced Feb 9, 2018

Preventing double nested links aidanwoods/parsedown#5

Merged

Housekeeping #553

Closed

Wrong parser #174

Closed

aidantwoods merged commit c192001 into erusev:master Feb 28, 2018

aidantwoods mentioned this pull request Feb 28, 2018

URL in [...](link) gets Double Linked #417

Closed

aidantwoods mentioned this pull request Feb 28, 2018

Double Parsing!! #504

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix Issue #358 – preventing double nested links #433

Fix Issue #358 – preventing double nested links #433

aidantwoods commented Oct 1, 2016 •

edited

aidantwoods commented Oct 1, 2016 •

edited

aidantwoods commented Oct 2, 2016 •

edited

aidantwoods commented Oct 4, 2016

PhrozenByte commented Oct 4, 2016 •

edited

aidantwoods commented Oct 4, 2016

PhrozenByte Oct 4, 2016

aidantwoods Oct 4, 2016

PhrozenByte Oct 4, 2016

PhrozenByte Oct 4, 2016

aidantwoods Oct 4, 2016

aidantwoods commented Oct 4, 2016

PhrozenByte commented Oct 4, 2016

aidantwoods commented Oct 8, 2016

erusev commented Oct 8, 2016 •

edited

aidantwoods commented Oct 8, 2016

erusev commented Oct 8, 2016 •

edited

aidantwoods commented Oct 8, 2016

erusev commented Oct 8, 2016

hatrena commented May 18, 2017

erusev commented May 18, 2017

Fix Issue #358 – preventing double nested links #433

Fix Issue #358 – preventing double nested links #433

Conversation

aidantwoods commented Oct 1, 2016 • edited

aidantwoods commented Oct 1, 2016 • edited

aidantwoods commented Oct 2, 2016 • edited

aidantwoods commented Oct 4, 2016

PhrozenByte commented Oct 4, 2016 • edited

aidantwoods commented Oct 4, 2016

PhrozenByte Oct 4, 2016

Choose a reason for hiding this comment

aidantwoods Oct 4, 2016

Choose a reason for hiding this comment

PhrozenByte Oct 4, 2016

Choose a reason for hiding this comment

PhrozenByte Oct 4, 2016

Choose a reason for hiding this comment

aidantwoods Oct 4, 2016

Choose a reason for hiding this comment

aidantwoods commented Oct 4, 2016

PhrozenByte commented Oct 4, 2016

aidantwoods commented Oct 8, 2016

erusev commented Oct 8, 2016 • edited

aidantwoods commented Oct 8, 2016

erusev commented Oct 8, 2016 • edited

aidantwoods commented Oct 8, 2016

erusev commented Oct 8, 2016

hatrena commented May 18, 2017

erusev commented May 18, 2017

aidantwoods commented Oct 1, 2016 •

edited

aidantwoods commented Oct 1, 2016 •

edited

aidantwoods commented Oct 2, 2016 •

edited

PhrozenByte commented Oct 4, 2016 •

edited

erusev commented Oct 8, 2016 •

edited

erusev commented Oct 8, 2016 •

edited