プロジェクト

全般

プロフィール

Vote #63565

完了

http links containing parentheses fail to reder correctly

Admin Redmine さんが3年以上前に追加. 3年以上前に更新.

ステータス:
Closed
優先度:
通常
担当者:
-
カテゴリ:
-
対象バージョン:
開始日:
2008/07/07
期日:
進捗率:

0%

予定工数:
category_id:
0
version_id:
2
issue_org_id:
1591
author_id:
278
assigned_to_id:
1
comments:
14
status_id:
5
tracker_id:
1
plus1:
0
affected_version:
closed_on:
affected_version_id:
5
ステータス-->[Closed]


journals

Here is a patch that should solve the problem. The regexp come from Rails with some modifications.
--------------------------------------------------------------------------------
Thanks for the patch.
But it would be nice to fix @RedCloth#inline_textile_link@ as well.
--------------------------------------------------------------------------------
This patch doesn't work well with links inside parenthesis. Eg. (http://www.foo.bar/Test_foobar).
Trailing parenthesis is included in the url.
--------------------------------------------------------------------------------
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 ) ?

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

<pre>
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
</pre>
--------------------------------------------------------------------------------
Jean-Philippe Lang wrote:
> Thanks for the patch.
> But it would be nice to fix @RedCloth#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 ?
--------------------------------------------------------------------------------
> 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 ?
--------------------------------------------------------------------------------
> 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
--------------------------------------------------------------------------------
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 :
<pre>
"If the url ends a sentence in parenthesis, don't include the closing parenthesis in the URL"
</pre>

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 :
<pre>
"If the url last char is closing ')' that is unbalanced in the url itself, take the ')' as external to the url
</pre>

This 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 ?
--------------------------------------------------------------------------------
> "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.
--------------------------------------------------------------------------------
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.
--------------------------------------------------------------------------------
jean philippe, are you reasonably happy with this patch ? If not, could you explain how you think it should be improved please.

--------------------------------------------------------------------------------
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.
--------------------------------------------------------------------------------
Paul, I didn't see your new patch, sorry.
Anyway, everything looks fine and it's now committed in r1871.

Thanks !
--------------------------------------------------------------------------------


related_issues

duplicates,Closed,1831,Textile bug in forming URL with last ')' symbol

Admin Redmine さんが3年以上前に更新

  • 対象バージョン0.8_2 にセット

他の形式にエクスポート: Atom PDF

いいね!0
いいね!0