Allow swapping built-in renderers without using their fully qualified names

[nicolus]

The documentation implies that the main purpose of Environment::addBlockRenderer() and Environment::addInlineRenderer() are to override the default renderers ("When registering these with the environment, you must tell it which block/inline classes it should handle. This allows you to essentially “swap out” built-in renderers with your own."), and the examples suggest that it can be done like this :

$environment->addBlockRenderer('FencedCode', $myRenderer);
$environment->addBlockRenderer('IndentedCode', $myRenderer);

The problem is this doesn't work, as you would actually have to do this in order to replace the built-in block renderers :

$environment->addBlockRenderer('League\CommonMark\Block\Element\FencedCode', $myRenderer);
$environment->addBlockRenderer('League\CommonMark\Block\Element\IndentedCode', $myRenderer);

The proposed pull requests adds tests that failed for both block and inline renderers being swapped as in the documentation, as well as a fix that makes it work, without breaking the existing tests which add other renderers.

It may not be the best way to work around that (I'm not even sure we should work around it), but at least the documentation should be consistent with the actual behavior.

Thanks.

[colinodell]

Thanks for submitting this! I agree, the documentation should absolutely be consistent with the library's behavior.

Let me think on this and I'll get back to you :)

[colinodell]

I'm wondering if we should switch to using simple class names instead (no namespaces). I haven't really thought through the potential consequences of that, but it would make registration easier.

If we don't go that route, then I think your idea makes sense. I wonder if we should use class_exists to check for CommonMark classes instead of hard-coding them. Thoughts?

[nicolus]

Thanks for taking time to review this.

I'm not sure how using class names without namespaces would work, but I'm afraid it might backfire. I think it would break code currently using the full namespace, and not allow things like this (with PHP 5.5+) :

use League\CommonMark\Block\Element\FencedCode;
//[...]
$environment->addBlockRenderer(FencedCode::class, $myRenderer);

Using class_exists sounds way better than my hard-coded array.

[colinodell]

I think that makes sense, especially the 5.5+ usage.

So in that case, if you want to modify your code to use class_exists instead then I'll go ahead and merge this in :)

[nicolus]

OK, I'm not sure why scrutinizer is yelling at me :-/

Anyway, feel free to make the change yourself if you need.

[colinodell]

Yeah, Scrutinizer has been doing that lately, not sure why. It looks good to me though :)

Thanks so much for implementing this!