On 2020-10-29 01:27, John Rouillard wrote:
> Is this patch supposed to create links for emails as well?

Yes.

> +        for group in ['url', 'email']:
> +            if match.group(group):
> 
> If so the patch is missing the email test cases.

I do not see the point to add a test that will test the exact same code.

> (I am still mystified what this code does and how it works.
> Is it just taking some random re/match and looking for a named group?

It does the same as for plain rendering but it takes care of links that
are already surrounded by <> or ().

> Then it uses the presence of the group to determine what was matched?)

Yes, the regexp defines some group.

> Also I had to modify the tests a bit. Markdown doesn't include a 
> trailing \n on the rendered lines, so I am using m.rstrip('\n') and
> removing the trailing \n. Also where you have self.assertIn I added:
> 
>  '<p><img alt="" src="http://example.com/" /></p>',
> 
> for markdown (apparently it sorts attributes by name).

Indeed this is a pain to have to support three markdown rendering.
I do not see why it is like that.
Also it would be better to test the markdown source instead of the
markdown rendering result because it is the source roundup is working
on.