Extending a core extension (SmartPunct) for LaTeX output

[samwilson]

I'm working on an extension for converting Markdown to LaTeX, and am trying to integrate the Smart Punctuation extension: samwilson/commonmark-latex#12

The classes in League\CommonMark\Extension\SmartPunct are all final and so can't be extended. I don't need to change anything other than the opening and closing quote characters, and the ellipsis rendering. It also needs to prevent SmartPunct from handling dashes.

Currently, it looks like the easiest way to do this would be to

1. prevent both the LatexRendererExtension and SmartPunctExtension from being loaded at the same time; and
2. duplicate the functionality of SmartPunct within the LaTeX extension, leaving out dashes.

However, this currently means actually duplicating the contents of the EllipsesParser, QuoteParser, QuoteProcessor, and ReplaceUnpairedQuotesListener classes. If these were not final, then they could just be extended.

I'm perhaps missing a better way of doing this though. Does anyone have any ideas?

Thanks!

[colinodell]

Hey there!

We generally follow https://ocramius.github.io/blog/when-to-declare-classes-final/ when deciding to make things final. For the SmartPunct extension:

• All classes implement an interface
• There are zero non-interface public methods
• There are zero protected methods
• Only QuoteParser has a (single) private method

If these were not final, then they could just be extended.

True, but even if we removed final from all of those classes, you'd still need to duplicate their public methods to change their behavior.

Have you considered using the decorator pattern? For example, to:

change [...] the opening and closing quote characters

You could wrap/decorate the QuoteParser like this:

final class CustomLatexParser implements InlineParserInterface {
    private InlineParserInterface $inner;

    public function __construct() {
        $this->inner = new QuoteParser();
    }

    public function getMatchDefinition(): InlineParserMatch {
        // TODO change this to fit your needs
        return InlineParserMatch::oneOf(Quote::SINGLE_QUOTE, Quote::DOUBLE_QUOTE);
    }

    public function parse(InlineParserContext $inlineContext): bool {
        return $this->inner->parse($inlineContext);
    }
}

Or like this:

final class CustomLatexParser implements InlineParserInterface {
    public function getMatchDefinition(): InlineParserMatch {
        // TODO change this to fit your needs
        return InlineParserMatch::oneOf(Quote::SINGLE_QUOTE, Quote::DOUBLE_QUOTE);
    }

    public function parse(InlineParserContext $inlineContext): bool {
        return (new QuoteParser)->parse($inlineContext);
    }
}

Both are effectively the same as:

final class CustomLatexParser extends QuoteParser {
    public function getMatchDefinition(): InlineParserMatch {
        // TODO change this to fit your needs
        return InlineParserMatch::oneOf(Quote::SINGLE_QUOTE, Quote::DOUBLE_QUOTE);
    }
}

Because you're using one public method from the original class as-is and providing your own implementation for the other.

Is that something you'd be open to trying? Or is there some use case that you feel doesn't work well with this approach?

[samwilson]

Thanks for the ideas! That does look like a good way to go. It almost works, but QuoteParser::parse() also creates a new Quote object, with $node = new Quote($char, ['delim' => true]), which isn't my Quote class and so I end up with an error of "Unable to find corresponding renderer for node type League\CommonMark\Extension\SmartPunct\Quote".

I'm perhaps not thinking about it correctly, but do you think it'd make sense for it to be possible to change the class name that's used in QuoteParser? e.g. the following in League\CommonMark\Extension\SmartPunct\QuoteParser:

    private string $quoteClass;
    public function __construct() {
        $this->quoteClass = Quote::class;
    }
    public function setQuoteClass(string $quoteClass) {
        $this->quoteClass = $quoteClass;
    }
    public function parse(InlineParserContext $inlineContext): bool {
        // …
        $node = new $this->quoteClass($char, ['delim' => true]);
        // …
    }

Similarly for ReplaceUnpairedQuotesListener.