Hooks instead of override?

[jerry1970]

In my extension on MarkdownExtra I have to copy a lot of methods only to change a single line.

Sometimes I have changed the regex a little bit (attributes on an image by reference), sometimes I want to add attributes (target=blank on external links), sometimes I want to change the output (img is wrapped in ).

In all of these cases, having a hook that is called would suffice. If several functions would check if a hook function is added I wouldn't have to copy a lot of code.

For example, this is the beginning of the inline anchors callback:

	protected function _doAnchors_inline_callback($matches) {
		$whole_match	=  $matches[1];
		$link_text		=  $this->runSpanGamut($matches[2]);
		$url			=  $matches[3] == '' ? $matches[4] : $matches[3];
		$title			=& $matches[7];
		$attr  = $this->doExtraAttributes("a", $dummy =& $matches[8]);

If this would be like this:

	protected function _doAnchors_inline_callback($matches) {
		$whole_match	=  $matches[1];
		$link_text		=  $this->runSpanGamut($matches[2]);
		$url			=  $matches[3] == '' ? $matches[4] : $matches[3];
		$title			=& $matches[7];
		$attr  =  $matches[8];
		if ($this->hooks['beforeAnchorsInline']) {
			$this->hooks['beforeAnchorsInline']($whole_match, $link_text, $url, $title, $attr);
		}
		$attr  = $this->doExtraAttributes("a", $attr);

And from my script (no extension needed, could be a wrapper), I could do this:

$parser->addHook('beforeAnchorsInline', function(&$whole, &$text, &$url, &$title, &$attr) {
	if (preg_match('//', $url)) {
		$attr .= " target=blank";
	}
};

This way I wouldn't have to copy any code, I can add the same hook to beforeAnchorInline and beforeAnchorByReference to save duplicate code.

When I have finished my current project, I could fork this project and work on it and make pull requests if you prefer.

[michelf]

There's already a few hooks for very specific things. Look at the configuration variables that are functions: url_filter_func, header_id_func, code_block_content_func, code_span_content_func. They all are hooks similar to yours.

I don't think hooks like that are the right approach long term however. One problem is that there is an infinite number of things people wants the output to be customized, so we'd need hooks everywhere. The other problem is each hook we add makes the parser code more complicated and this increases maintenance cost.

Ideally, I think the HTML output should be generated by a separate class. For instance, we could have something like that:

class MarkdownOutput {
    function generateAnchor($content, $url, $title, $classes, $attributes) { ... }
    // ...other HTML generating functions here...
}

Then we make MarkdownExtra call the appropriate method on a MarkdownOutput object to get the HTML required for the anchor. _doAnchors_inline_callback could be simplified to something like this:

protected function _doAnchors_inline_callback($matches) {
	$whole_match	=  $matches[1];
	$link_text		=  $this->runSpanGamut($matches[2]);
	$url			=  $matches[3] == '' ? $matches[4] : $matches[3];
	$title			=& $matches[7];
	$attr  = $this->doExtraAttributes("a", $dummy =& $matches[8]);
	// if the URL was of the form <s p a c e s> it got caught by the HTML
	// tag parser and hashed. Need to reverse the process before using the URL.
	$unhashed = $this->unhash($url);
	if ($unhashed != $url)
		$url = preg_replace('/^<(.*)>$/', '\1', $unhashed);
	$link_text = $this->runSpanGamut($link_text);
	$result = $output->generateAnchor($link_text, $url, $title, $attr);
	return $this->hashPart($result);
}

With this approach we're introducing a separation of concerns: one class deals with the parsing, the other class deals with generating the HTML, and I think that'd be much more healthy than adding a myriad of hooks everywhere. If you truly want to pursue a general solution to the problem of customizing the output, I suggest you investigate this avenue.

I think that'll even help a bit for cases where you want to change the regex, because it'll be easy to hook into the same HTML generating code once you've done your custom parsing.