Refactor and improve formatter extender (#2098)

- Deprecated all events involved with Formatter
- Refactor ->configure() method on extender not to use events
- Add extender methods for ->render() and ->parse()
- Add integration tests

framework/core/src/Extend/Formatter.php

@@ -10,38 +10,104 @@
 namespace Flarum\Extend;
 
 use Flarum\Extension\Extension;
-use Flarum\Formatter\Event\Configuring;
 use Flarum\Formatter\Formatter as ActualFormatter;
 use Illuminate\Contracts\Container\Container;
-use Illuminate\Events\Dispatcher;
 
 class Formatter implements ExtenderInterface, LifecycleInterface
 {
-    private $callback;
+    private $configurationCallbacks = [];
+    private $parsingCallbacks = [];
+    private $renderingCallbacks = [];
 
+    /**
+     * Configure the formatter. This can be used to add support for custom markdown/bbcode/etc tags,
+     * or otherwise change the formatter. Please see documentation for the s9e text formatter library for more
+     * information on how to use this.
+     *
+     * @param callable|string $callback
+     *
+     * The callback can be a closure or invokable class, and should accept:
+     * - \s9e\TextFormatter\Configurator $configurator
+     */
     public function configure($callback)
     {
-        $this->callback = $callback;
+        $this->configurationCallbacks[] = $callback;
+
+        return $this;
+    }
+
+    /**
+     * Prepare the system for parsing. This can be used to modify the text that will be parsed, or to modify the parser.
+     * Please note that the text to be parsed must be returned, regardless of whether it's changed.
+     *
+     * @param callable|string $callback
+     *
+     * The callback can be a closure or invokable class, and should accept:
+     * - \s9e\TextFormatter\Parser $parser
+     * - mixed $context
+     * - string $text: The text to be parsed.
+     *
+     * The callback should return:
+     * - string $text: The text to be parsed.
+     */
+    public function parse($callback)
+    {
+        $this->parsingCallbacks[] = $callback;
+
+        return $this;
+    }
+
+    /**
+     * Prepare the system for rendering. This can be used to modify the xml that will be rendered, or to modify the renderer.
+     * Please note that the xml to be rendered must be returned, regardless of whether it's changed.
+     *
+     * @param callable|string $callback
+     *
+     * The callback can be a closure or invokable class, and should accept:
+     * - \s9e\TextFormatter\Rendered $renderer
+     * - mixed $context
+     * - string $xml: The xml to be rendered.
+     * - ServerRequestInterface $request
+     *
+     * The callback should return:
+     * - string $xml: The xml to be rendered.
+     */
+    public function render($callback)
+    {
+        $this->renderingCallbacks[] = $callback;
 
         return $this;
     }
 
     public function extend(Container $container, Extension $extension = null)
     {
-        $events = $container->make(Dispatcher::class);
-
-        $events->listen(
-            Configuring::class,
-            function (Configuring $event) use ($container) {
-                if (is_string($this->callback)) {
-                    $callback = $container->make($this->callback);
-                } else {
-                    $callback = $this->callback;
+        $container->extend('flarum.formatter', function ($formatter, $container) {
+            foreach ($this->configurationCallbacks as $callback) {
+                if (is_string($callback)) {
+                    $callback = $container->make($callback);
                 }
 
-                $callback($event->configurator);
+                $formatter->addConfigurationCallback($callback);
             }
-        );
+
+            foreach ($this->parsingCallbacks as $callback) {
+                if (is_string($callback)) {
+                    $callback = $container->make($callback);
+                }
+
+                $formatter->addParsingCallback($callback);
+            }
+
+            foreach ($this->renderingCallbacks as $callback) {
+                if (is_string($callback)) {
+                    $callback = $container->make($callback);
+                }
+
+                $formatter->addRenderingCallback($callback);
+            }
+
+            return $formatter;
+        });
     }
 
     public function onEnable(Container $container, Extension $extension)

framework/core/src/Formatter/Event/Configuring.php

@@ -11,6 +11,9 @@ namespace Flarum\Formatter\Event;
 
 use s9e\TextFormatter\Configurator;
 
+/**
+ * @deprecated beta 15, removed beta 16. Use the Formatter extender instead.
+ */
 class Configuring
 {
     /**

framework/core/src/Formatter/Event/Parsing.php

@@ -11,6 +11,9 @@ namespace Flarum\Formatter\Event;
 
 use s9e\TextFormatter\Parser;
 
+/**
+ * @deprecated beta 15, removed beta 16. Use the Formatter extender instead.
+ */
 class Parsing
 {
     /**

framework/core/src/Formatter/Event/Rendering.php

@@ -12,6 +12,9 @@ namespace Flarum\Formatter\Event;
 use Psr\Http\Message\ServerRequestInterface;
 use s9e\TextFormatter\Renderer;
 
+/**
+ * @deprecated beta 15, removed beta 16. Use the Formatter extender instead.
+ */
 class Rendering
 {
     /**

framework/core/src/Formatter/Formatter.php

@@ -20,6 +20,12 @@ use s9e\TextFormatter\Unparser;
 
 class Formatter
 {
+    protected $configurationCallbacks = [];
+
+    protected $parsingCallbacks = [];
+
+    protected $renderingCallbacks = [];
+
     /**
      * @var Repository
      */
@@ -47,6 +53,21 @@ class Formatter
         $this->cacheDir = $cacheDir;
     }
 
+    public function addConfigurationCallback($callback)
+    {
+        $this->configurationCallbacks[] = $callback;
+    }
+
+    public function addParsingCallback($callback)
+    {
+        $this->parsingCallbacks[] = $callback;
+    }
+
+    public function addRenderingCallback($callback)
+    {
+        $this->renderingCallbacks[] = $callback;
+    }
+
     /**
      * Parse text.
      *
@@ -58,8 +79,13 @@ class Formatter
     {
         $parser = $this->getParser($context);
 
+        // Deprecated in beta 15, remove in beta 16
         $this->events->dispatch(new Parsing($parser, $context, $text));
 
+        foreach ($this->parsingCallbacks as $callback) {
+            $text = $callback($parser, $context, $text);
+        }
+
         return $parser->parse($text);
     }
 
@@ -75,8 +101,13 @@ class Formatter
     {
         $renderer = $this->getRenderer();
 
+        // Deprecated in beta 15, remove in beta 16
         $this->events->dispatch(new Rendering($renderer, $context, $xml, $request));
 
+        foreach ($this->renderingCallbacks as $callback) {
+            $xml = $callback($renderer, $context, $xml, $request);
+        }
+
         return $renderer->render($xml);
     }
 
@@ -122,8 +153,13 @@ class Formatter
         $configurator->Autolink;
         $configurator->tags->onDuplicate('replace');
 
+        // Deprecated in beta 15, remove in beta 16
         $this->events->dispatch(new Configuring($configurator));
 
+        foreach ($this->configurationCallbacks as $callback) {
+            $callback($configurator);
+        }
+
         $this->configureExternalLinks($configurator);
 
         return $configurator;

framework/core/tests/integration/extenders/FormatterTest.php

@@ -0,0 +1,145 @@
+<?php
+
+/*
+ * This file is part of Flarum.
+ *
+ * For detailed copyright and license information, please view the
+ * LICENSE file that was distributed with this source code.
+ */
+
+namespace Flarum\Tests\integration\extenders;
+
+use Flarum\Extend;
+use Flarum\Formatter\Formatter;
+use Flarum\Tests\integration\TestCase;
+
+class FormatterTest extends TestCase
+{
+    protected function getFormatter()
+    {
+        $formatter = $this->app()->getContainer()->make(Formatter::class);
+        $formatter->flush();
+
+        return $formatter;
+    }
+
+    /**
+     * @test
+     */
+    public function custom_formatter_config_doesnt_work_by_default()
+    {
+        $formatter = $this->getFormatter();
+
+        $this->assertEquals('<t>[B]something[/B]</t>', $formatter->parse('[B]something[/B]'));
+    }
+
+    /**
+     * @test
+     */
+    public function custom_formatter_config_works_if_added_with_closure()
+    {
+        $this->extend((new Extend\Formatter)->configure(function ($config) {
+            $config->BBCodes->addFromRepository('B');
+        }));
+
+        $formatter = $this->getFormatter();
+
+        $this->assertEquals('<b>something</b>', $formatter->render($formatter->parse('[B]something[/B]')));
+    }
+
+    /**
+     * @test
+     */
+    public function custom_formatter_config_works_if_added_with_invokable_class()
+    {
+        $this->extend((new Extend\Formatter)->configure(InvokableConfig::class));
+
+        $formatter = $this->getFormatter();
+
+        $this->assertEquals('<b>something</b>', $formatter->render($formatter->parse('[B]something[/B]')));
+    }
+
+    /**
+     * @test
+     */
+    public function custom_formatter_parsing_doesnt_work_by_default()
+    {
+        $this->assertEquals('<t>Text&lt;a&gt;</t>', $this->getFormatter()->parse('Text<a>'));
+    }
+
+    /**
+     * @test
+     */
+    public function custom_formatter_parsing_works_if_added_with_closure()
+    {
+        $this->extend((new Extend\Formatter)->parse(function ($parser, $context, $text) {
+            return 'ReplacedText<a>';
+        }));
+
+        $this->assertEquals('<t>ReplacedText&lt;a&gt;</t>', $this->getFormatter()->parse('Text<a>'));
+    }
+
+    /**
+     * @test
+     */
+    public function custom_formatter_parsing_works_if_added_with_invokable_class()
+    {
+        $this->extend((new Extend\Formatter)->parse(InvokableParsing::class));
+
+        $this->assertEquals('<t>ReplacedText&lt;a&gt;</t>', $this->getFormatter()->parse('Text<a>'));
+    }
+
+    /**
+     * @test
+     */
+    public function custom_formatter_rendering_doesnt_work_by_default()
+    {
+        $this->assertEquals('Text', $this->getFormatter()->render('<p>Text</p>'));
+    }
+
+    /**
+     * @test
+     */
+    public function custom_formatter_rendering_works_if_added_with_closure()
+    {
+        $this->extend((new Extend\Formatter)->render(function ($renderer, $context, $xml, $request) {
+            return '<html>ReplacedText</html>';
+        }));
+
+        $this->assertEquals('ReplacedText', $this->getFormatter()->render('<html>Text</html>'));
+    }
+
+    /**
+     * @test
+     */
+    public function custom_formatter_rendering_works_if_added_with_invokable_class()
+    {
+        $this->extend((new Extend\Formatter)->render(InvokableRendering::class));
+
+        $this->assertEquals('ReplacedText', $this->getFormatter()->render('<html>Text</html>'));
+    }
+}
+
+class InvokableConfig
+{
+    public function __invoke($config)
+    {
+        $config->BBCodes->addFromRepository('B');
+    }
+}
+
+class InvokableParsing
+{
+    public function __invoke($parser, $context, $text)
+    {
+        return 'ReplacedText<a>';
+    }
+}
+
+class InvokableRendering
+{
+    public function __invoke($renderer, $context, $xml, $request)
+    {
+        return '<html>ReplacedText</html>';
+    }
+}