Defect #1591
closedhttp links containing parentheses fail to reder correctly
0%
Description
for example:
http://en.wikipedia.org/wiki/Reflection_(computer_science)
http://en.wikipedia.org/wiki/Introspection_(computer_science)
http://en.wikipedia.org/wiki/Introspector_(program)
will be rendered without right parenthesis.
Files
Related issues
Updated by Pierre Paysant-Le Roux over 16 years ago
Here is a patch that should solve the problem. The regexp come from Rails with some modifications.
Updated by Jean-Philippe Lang over 16 years ago
Thanks for the patch.
But it would be nice to fix RedCloth#inline_textile_link
as well.
Updated by Jean-Philippe Lang about 16 years ago
This patch doesn't work well with links inside parenthesis. Eg. (http://www.foo.bar/Test_foobar).
Trailing parenthesis is included in the url.
Updated by Paul Rivier about 16 years ago
- Assignee set to Paul Rivier
Jean-Philippe Lang wrote:
This patch doesn't work well with links inside parenthesis. Eg. (http://www.foo.bar/Test_foobar).
Trailing parenthesis is included in the url.
This is not an easy problem. Should we consider that parenthesis inside url should alway be balanced ? I don't think we should.
Then should we treat the particular case of a url with a preceding ( and a trailing ) ?
Updated by Paul Rivier about 16 years ago
Paul Rivier wrote:
This is not an easy problem. Should we consider that parenthesis inside url should alway be balanced ? I don't think we should.
Then should we treat the particular case of a url with a preceding ( and a trailing ) ?
here is a naive patch for the later.
def inline_auto_link(text) text.gsub!(AUTO_LINK_RE) do all, a, b, c, d = $&, $1, $2, $3, $4 if a =~ /<a\s/i || a =~ /![<>=]?/ else ################## PATCH BELOW if ( a[-1] == ?( and c[-1]==?) ) # looks like url is put inside parenthesis c=c[0..-2] d=")"+d end ################## PATCH ABOVE text = b + c %(#{a}<a class="external" href="#{b=="www."?"http://www.":b}#{c}">#{text}</a>#{d}) end end end
Updated by Paul Rivier about 16 years ago
Jean-Philippe Lang wrote:
Thanks for the patch.
But it would be nice to fixRedCloth#inline_textile_link
as well.
What's the policy for this kind of "bundled-librairy" related problem ? Should we patch against bundled files ? Should we try to monkey-patch from outside ?
Updated by Jean-Philippe Lang about 16 years ago
Should we consider that parenthesis inside url should alway be balanced ? I don't think we should.
I don't know which is the best solution but I'd really like that something like this works: (see: http://example.net).
I think we see this more often than a link with a single closing parenthesis at the end.
WDYT ?
Updated by Jean-Philippe Lang about 16 years ago
Should we patch against bundled files ?
Yes, you can.redcloth.rb
is already patched: http://www.redmine.org/repositories/changes/redmine/trunk/lib/redcloth.rb
Updated by Paul Rivier about 16 years ago
Jean-Philippe Lang wrote:
I don't know which is the best solution but I'd really like that something like this works: (see: http://example.net).
I think we see this more often than a link with a single closing parenthesis at the end.
WDYT ?
For sure I'd like this as well, but we first have to formulate clearly what we want. For the moment, it's like :
"If the url ends a sentence in parenthesis, don't include the closing parenthesis in the URL"
That would involve matching the whole sentence, before url to see if it starts with '(' and after to see if there isn't an other closing ')' later, like in "(see: http://fr.example.net/wiki/clavier_(musique) to read more)". I think we will have hard time doing so :/
Maybe we can do that :
"If the url last char is closing ')' that is unbalanced in the url itself, take the ')' as external to the urlThis seems doable, does not require to scan far around the url itself, and should handle :
- (see: http://example.net)
- (see: http://fr.example.net/wiki/clavier_(musique) to read more)
- (see: http://fr.example.net/wiki/clavier_(musique))
WDYT ?
Updated by Jean-Philippe Lang about 16 years ago
"If the url last char is closing ')' that is unbalanced in the url itself, take the ')' as external to the url
This is fine for me.
Updated by Paul Rivier about 16 years ago
- File urls_fix.patch urls_fix.patch added
Jean-Philippe Lang wrote:
"If the url last char is closing ')' that is unbalanced in the url itself, take the ')' as external to the url
This is fine for me.
so please find attached patch against current trunk. It's fairly minor, and does not change the (already fairly complex) regexp for url except concerning the parenthesis. Tests included.
Updated by Paul Rivier about 16 years ago
jean philippe, are you reasonably happy with this patch ? If not, could you explain how you think it should be improved please.
Updated by Paul Rivier about 16 years ago
- Status changed from New to 7
- Assignee changed from Paul Rivier to Jean-Philippe Lang
What's up with this patch please ?
Jean Philippe, you are the new assignee, as you may want to apply it or not, or close the issue.
Updated by Jean-Philippe Lang about 16 years ago
- Status changed from 7 to Closed
- Target version set to 0.8
- Affected version (unused) set to 0.7.3
- Resolution set to Fixed
- Affected version set to 0.7.3
Paul, I didn't see your new patch, sorry.
Anyway, everything looks fine and it's now committed in r1871.
Thanks !