make_clickable shouldn't include trailing punctuation

There are instances where that trailing period "should" be linked.

If I look at the RFC, the link www.wordpress.org. (with the trailing dot) is a valid FQDN and in theorie the period should therefore also be linked.

I see the point of unlinking the trailing period, but wanted to make this side-effect clear.

Well, "www.wordpress.org." is not a URL. If it were a real URL, like the following, it would be have a trailing slash: http://www.wordpress.org./

But, I guess, if for some reason the URL required a full stop at the end of it, you'd get problems. You could simply override make_clickable() by using a manual <a href="http://example.com/wacko.">http://example.com/wacko.</a>. Having a full stop at the end of the URL seems like a really niche case.

Definitely a niche case (valid URL ending in a "dot"). A lot of "make clickable" implementations screw this up and put things like ending parethesis and ending "dots" into the href, so it would be a refreshing change if we could make our implementation handle those cases better.

Replying to Nazgul:

There are instances where that trailing period "should" be linked. If I look at the RFC, the link www.wordpress.org. (with the trailing dot) is a valid FQDN and in theorie the period should therefore also be linked. I see the point of unlinking the trailing period, but wanted to make this side-effect clear.

I think in 99.9% of cases, a period at the end of the URL is just that -- a period and not part of the URL. It shouldn't be included.

Replying to Viper007Bond:

I think in 99.9% of cases, a period at the end of the URL is just that -- a period and not part of the URL. It shouldn't be included.

I completely agree.

It's just that I had a similar issue in a program at work and got quite some negative feedback when I made that change (picky customer), so I thought I'd at least throw it out there. Now it's a design decision instead of an unknown side-effect. :)

removes .,;: from urls with a protocol, [,;:] from urls without a protocol

Added patch. It removes a trailing period, comma, colon or semicolon in URLs with a protocol (i.e. http://wordpress.org), but only the trailing comma, colon or semicolon in URLs without a protocol (i.e. www.wordpress.org). The consensus seemed indicate that www.wordpress.org. is a FQDN, so should stay as is.

btw - this is my first attempt at fixing a bug; advance apologies for making any newbie mistakes!

I think we can strip the valid but only informative . off the end of domain names being made clickable as having it/not having it shouldn't make any difference to how the link is interpreted.

The patch looks good - I think it would be good to add some tests to the unit test suite for this as well to prove it works well for all cases.

Test cases; diff against revision 170 at http://svn.automattic.com/wordpress-tests/

Just attached the (2) unit tests, as against the wordpress-tests svn. Just running revision 170 of wp-tests against [7343] gives 31 failures and 13 errors (though some are because of safe mode restrictions or bugs in the unit tests themselves). After applying the formatting.php.diff to wordpress and the just-attached patch to wordpress-tests, it gives the same 31/13 failures, but two more successes, so I'd assume my tests passed :)

And hence, I suppose we can call this bug fixed?

Bugs are marked as fixed once the fix has been checked in. I'm heading to bed now, but will take a look at it tomorrow.

I see! Good to know. Will someone commit the test case into the tests svn as well, once the fix has been checked in?

neodude, you can run only your tests by executing: php wp-test.php -t <test-classname-here>

Also, we have a system, which can connect your unit test with WordPress trac tickets. If you add:

$this->knownWPBug(5081);

call in your the beginning of a test method, it will:

• be skipped if the corresponding ticket isn't closed and checked otherwise
• be skipped if -s command line option is supplied for wp-test.php
• be checked if -f command line option is supplied for wp-test.php

Your tests were committed in revision 172 of wordpress-tests.

This is extremely useful information - probably would've saved me hours of fiddling with wordpress-tests. Is this testing framework documented anywhere?

Replying to neodude:

This is extremely useful information - probably would've saved me hours of fiddling with wordpress-tests. Is this testing framework documented anywhere?

You want better documentation than the code itself? ;) No, it isn't. Have a look at {{{wp-test.php}}, it is pretty straightforward.

(In [7452]) make_clickable() trailing punctuation fixes from neodude. fixes #5081

FYI, I committed it with it omitting the trailing "dot" -- if you want a trailing dot in a hyperlink, you can code it up properly. In the majority of cases, we don't want it.