textile / php-textile

Textile markup language parser for PHP
https://textile-lang.com
Other
216 stars 44 forks source link

Allow inline links like "test?param[]=value" #87

Closed trendfischer closed 11 years ago

trendfischer commented 12 years ago

Tested with the following textile snippets:

Syntax that should work still after the patch:

Try it out on "github":http://github.com.

...blah blah (for more details look on "github":http://github.com) blah blah...

...blah blah [for more details look on "github":http://github.com] blah blah...

...blah blah[^["on github":http://github.com]^] blah...

...blah blah ^["on github":http://github.com]^ blah...

Syntax that did not work before:

"my link":http://github.com?param[]=value

In a sentence "my link":http://github.com?param[]=value.

In a sentence "my link":http://github.com?param[].

...blah blah ^["on github":http://github.com?param[]]^ blah...
netcarver commented 12 years ago

I've had to modify your patch slightly but have now pushed it to a new issue-86 test branch. Could you try this version of classTextile.php and see if it does everything you need it to. Thanks!

netcarver commented 12 years ago

I've been doing some follow-up tests on this today and it's worth noting that what I've committed to the issue-86 branch will only work for array parameters like ?q[]=1&q[]=2 but if you try using indexes it will fail eg. ?q[one]=1&q[two]=2 won't work.

trendfischer commented 12 years ago

I guess we can close this pull request with the changes here: https://raw.github.com/netcarver/textile/link-detection-rewrite/classTextile.php

netcarver commented 12 years ago

I'll leave it open as a reminder until I've actually committed something to the dev branch that will resolve this. I did find a problem with links like this...

"testing":

"He said it is "very unlikely" the "economic stimulus" works":http://slashdot.org/

""Open the pod bay doors please, HAL."":http://www.youtube.com/watch?v=npN9l2Bd06s 

...which all come from the Redcloth link tests and are either invalid or have embedded " characters within the text part of the link.

trendfischer commented 12 years ago

The first one "testing": That should not be a link, cause it is a valid usecase in a common sentence. The other examples are bit more tricky. Especially if you have further examples like this:

"The use of the characted «"» in textile":help.html

or

For "myparser" use this "link":help.html

For the first one, it fails if you count the quotes, for the second one you could not use a greedy regexp. So I wouldn't support these link examples from Redcloth. Supporting them might cause more problems than they solve.

netcarver commented 12 years ago

I updated my work-in-progress here. Feel free to try out your two examples from above and see what you think. For the second you should be able to do For ""myparser" use this "link"":help.html or "For "myparser" use this "link"":help.html.

netcarver commented 11 years ago

Closed by f13063beb5 in the master branch and 1c65b8b47a in the 2.5 branch.