Support Parentheses in URLs

[sakjur]

Bug description

The URL http://msdn.microsoft.com/en-us/library/windows/desktop/ms724451(v=vs.85).aspx must be written as http://msdn.microsoft.com/en-us/library/windows/desktop/ms724451%28v=vs.85%29.aspx for Snudown not to parse the second parenthesis as end of link.

Expected behaviour

The link http://msdn.microsoft.com/en-us/library/windows/desktop/ms724451(v=vs.85).aspx should work in Markdown as is. GitHub does it.

Steps to reproduce

Write a comment on Reddit containing the code

[GitHub does it](http://msdn.microsoft.com/en-us/library/windows/desktop/ms724451(v=vs.85).aspx)

should be equivalent to

[GitHub does it](http://msdn.microsoft.com/en-us/library/windows/desktop/ms724451%28v=vs.85%29.aspx)

Proposed fix

When a opening parenthesis is occuring within a URL according to the [link](url)-schema, a counter will tick up to one. If a closing parenthesis is occured, the counter will tick down by one. When the counter is below zero, the last parenthesis is not counted as part of the URL and the URL terminated.

Bug introduced by proposed fix

When having an opening parenthesis without a closing, the following code will make the URL weird
[GitHub does it](http://msdn.microsoft.com/en-us/library/windows/desktop/ms724451(v=vs.85.aspx)

I propose that if a parenthesis is immediately followed by a non-URL-safe character (e.g. space), it will assume that is a closing parenthesis despite being opened.

Thus, the following successful testcases needs to be introduced

1. [link](http://example.com/parenthe(sis).html)
1. [link](http://example.com/parenthe(sis.html) something else
1. [link](http://example.com/parenthe(sis.html))

The following testcases will probably fail:

1. [link](http://example.com/parenthesis).html)
1. [link](http://example.com/parenthe(sis.html). Something else.

This should still work:

1. [link](http://example.com/parenthe%28sis%29.html)

[xiongchiamiov]

I usually escape the close paren ([link](http://msdn.microsoft.com/en-us/library/windows/desktop/ms724451(v=vs.85\).aspx)). I run into this pretty frequently with Wikipedia articles.

There are rules on this in the CommonMark spec, so I suspect we'll be gradually moving towards compliance with that, although @spladug should be able to actually usefully comment on this.

[sakjur]

@xiongchiamiov I run Firefox, which encodes parenthesises on the fly, but I ran into this issue out in the wild, and I suppose most Redditors doesn't check their posts in that way before publishing.

It's a bit a sad how Markdown (by necessity) breaks RFC2396 for URLs, as that means more fragmentation to the way we write links. the (name)[link] might've been preferable in hindsight, but that never happened 😿

If the CommonMark spec is a proper (non-ambigious) standard, I find it sensible to follow the requirements it set up, despite this potentially breaking a page in ten thousand or so.

[JordanMilne]

I don't think that we'll change this in SnuDown proper, but we're looking at moving to a fork of https://github.com/github/cmark which implements open / closing paren counting in both the autolinking code and the inline link parsing.