Add support for Commonmark v2

Created on 31 May 2022, over 2 years ago
Updated 31 May 2023, over 1 year ago

Problem/Motivation

CommonMark 2 has been available for awhile now, but conflicts with Drush β†’ and other components have prevented upgrading. These issues have now been solved, and I successfully upgraded to commonmark 2.3.1 with composer. Of course, that immediately failed with an error because the Commonmark API has changed, and this module must be updated to support that.

Proposed resolution

Make a major new release of the module that supports Commonmark 2.

official docs for developers for updating to 2.x

Remaining tasks

User interface changes

API changes

Should we keep compatibility with Commonmark 1 and 2? Or should the new major release only support v2?

Data model changes

✨ Feature request
Status

Active

Version

3.0

Component

Code

Created by

πŸ‡―πŸ‡΅Japan ptmkenny

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

  • πŸ‡¬πŸ‡§United Kingdom robcarr Perthshire, Scotland

    There's a dependency on Commonmark 2 with the Migrate Markdown to HTML module ( https://www.drupal.org/project/migrate_process_markdown_to_html β†’ ), so I'd say this feature is quite important

  • Status changed to Needs review over 1 year ago
  • Open in Jenkins β†’ Open on Drupal.org β†’
    Core: 9.5.x + Environment: PHP 8.0 & MySQL 5.7
    last update over 1 year ago
    1 pass
  • πŸ‡­πŸ‡ΊHungary tikaszvince

    Hi,

    I have a very minimal patch, to prevent error on converting when we want to use commonmark 2.4.
    This patch can apply on 3.0.0-rc2 version of markdown module

  • πŸ‡ΊπŸ‡ΈUnited States jasonawant New Orleans, USA

    Hi @tikaszvince, thanks for the patch!

    Have you encountered any issues? I'm curious how you are using this module with Commonmark v2.

    A quick glance Commonmark 1.6 to 2.0 upgrade docs suggests other changes are required.

    Perhaps these are already in place when looking at https://git.drupalcode.org/project/markdown/-/blob/3.0.x/src/Plugin/Mark... from changes in #3142418: Support multiple libraries per plugin β†’ .

  • πŸ‡­πŸ‡ΊHungary tikaszvince

    I had=ve a problem with the approach checking which class exists. The Commonmark Environment should be constructed in different way with version 2.0 and there were some changes since 2.0 to 2.4. So i had to check for the version string.

    The more i think about this the more I'm convinced about the proper approach should be a different @MarkdownParser for newer Commonmark versions.

    My patch also uses the \ReflectionObject to merge settings which is against the intention of the Commonmark, which triggers a deprecation warning too when you call merge().

    I'm using the markdown module and Commonmark for convert texts from an API and store them in a formatted text field. In my case these were the required changes to make my integration code work after upgrades.

  • Status changed to Needs work over 1 year ago
  • πŸ‡©πŸ‡ͺGermany c-logemann Frankfurt/M, Germany

    I tried to get Commonmark 2.4 to run with this #5 patch and got still WSOD. Because upgrading Commonmark is currently not so important on the related project I downgraded again and just wanted to report here.

  • πŸ‡ΊπŸ‡ΈUnited States jasonawant New Orleans, USA

    Thanks tikaszvince for the notes and C-Logemann for reporting. I'm looking into these changes this week.

    @tikaszvince, I agree about a new plugin for Commonmark v2, but I do wonder about sharing the commonmark markdown extensions across both; will there also need to be v2 only extensions.

    The project requirement I'm hoping to satisfy utilizes Commonmark v2 ability to configure disallowed raw HTML tags, which will require an update to the commonmark-disallowed-raw-html markdown extension; a change not compatible with Commonmark v1.

    The v2's Commonmark::convertToHtml() will have to change per these update docs.

    From

      protected function convertToHtml($markdown, LanguageInterface $language = NULL) {
        return $this->converter()->convertToHtml($markdown);
      }
    

    to

      protected function convertToHtml($markdown, LanguageInterface $language = NULL) {
        return $this->converter()->convertToHtml($markdown)->getContent();
      }
    
    
  • πŸ‡ΊπŸ‡ΈUnited States jasonawant New Orleans, USA

    Hi,

    I've attached two files as work in progress: 1) commonmark-v1-v2.diff to show changes between the two versions and 2) a new patch to add CommonMarkV2.php MarkdownParser plugin. The second patch differs from the first patch by making an entirely new Commonmark v2 plugin.

    I have to switch to some other work briefly, but will return to this later today.

    Here's a list of next steps

    • Solve for max_nesting_level value type casting as string instead of integer; this results in an error: League\Config\Exception\ValidationException: The item 'max_nesting_level' expects to be int, '10' given. in League\Config\Configuration->build() (line 195 of /var/www/vendor/league/config/src/Configuration.php)
    • Solve for Error: RuntimeException: Unable to find corresponding renderer for node type League\CommonMark\Node\Block\Document in League\CommonMark\Renderer\HtmlRenderer->renderNode() (line 88 of /var/www/vendor/league/commonmark/src/Renderer/HtmlRenderer.php)
    • Check the Configuration Option Changes, @see: https://commonmark.thephpleague.com/2.0/upgrading/developers/#configurat...
    • Refactor max_nesting_level default value management instead of using PHP_INT_MAX
    • General code cleanup and refactoring
    • Updating CommonMark MarkdownParser to not include v2
    • Creating required CommonMark v2 Extensions; updating existing existing extensions to not include v2
  • Assigned to jasonawant
  • πŸ‡ΊπŸ‡ΈUnited States jasonawant New Orleans, USA

    While I work on this, I'll assign this issue to myself. I'll share some progress in the next few days.

  • Issue was unassigned.
  • πŸ‡ΊπŸ‡ΈUnited States jasonawant New Orleans, USA

    Attached is a new patch that solves the following error by creating a CommonMarkCoreExtension MarkdownExtension plugin. This probably isn't the best way to solve the error; I imagine CommonMarkCoreExtension should be added to the environment by default without enabling it as a MarkdownExtension plugin.

    Solve for Error: RuntimeException: Unable to find corresponding renderer for node type League\CommonMark\Node\Block\Document

    The patch also changes the existing Commonmark MarkdownExtension plugins InstallableRequirement constraints to prevent them from being used with the new CommonMarkV2 parser.

    And lastly, the patch includes a new configurable Disallowed Raw HTML MarkdownExtension plugin with CommonMarkV2 parser Installable Requirement constraint. This uses the v2 supported config options and satisfied my project requirement to remove tags.

    While I can render content using the CommonMarkV2 parser...HOWEVER, its not rendering using the Enable Emphasis, Enable Strong, Use Asterisk, Use Underscore or Unordered List Markers options.

    I need to check-in with my project team about the remaining effort and timing. I'm starting to think using a custom, or contributed, Drupal @Filter plugin that integrates directly with CommonMark V2 may be more reliable and maintainable path going forward. As much as I'd like to see this Markdown module work with CommonMark v2 for more broader usage, there's a lot of abstraction found within this Markdown module to account for.

  • πŸ‡ΊπŸ‡ΈUnited States jasonawant New Orleans, USA

    Well, we've pivoted away from using this module. Instead, we've implemented a custom Drupal @Filter plugin (see d.o. forum for related discussion on filter plugins β†’ ) to use CommonMark V2 directly.

    Here's an example of that implementation. In ours, I created custom extension and image renderer to replace commonmarkcore below and its processors and renderers to not render image markdown.

    namespace Drupal\my_module\Plugin\Filter;
    
    use Drupal\filter\FilterProcessResult;
    use Drupal\filter\Plugin\FilterBase;
    use Drupal\Core\Logger\LoggerChannelTrait;
    use League\CommonMark\Environment\Environment;
    use League\CommonMark\Extension\CommonMark\
    use League\CommonMark\Exception\CommonMarkException\CommonMarkCoreExtension;
    use League\CommonMark\Extension\Strikethrough\StrikethroughExtension;
    use League\CommonMark\MarkdownConverter;
    
    /**
     * CommonMark V2 filter plugin for Markdown.
     *
     * @Filter(
     *   id = "commonmark_v2",
     *   title = @Translation("CommonMark V2"),
     *   description = @Translation("Utilizes CommonMark V2 to parse markdown and convert into HTML"),
     *   type = Drupal\filter\Plugin\FilterInterface::TYPE_MARKUP_LANGUAGE,
     * )
     */
    class CommonMarkV2 extends FilterBase {
    
      use LoggerChannelTrait;
    
      /**
       * The configuration array.
       *
       * @see https://commonmark.thephpleague.com/configuration/.
       */
      private array $config = [
        'renderer' => [
          'block_separator' => "\n",
          'inner_separator' => "\n",
          'soft_break'      => "\n",
        ],
        'html_input' => 'strip',
        'allow_unsafe_links' => FALSE,
        'max_nesting_level' => 10,
      ];
    
      /**
       * {@inheritdoc}
       */
      public function process($text, $langcode): FilterProcessResult {
        try {
          $environment = $this->createEnvironment();
          $converter = new MarkdownConverter($environment);
          $converted_text = $converter->convert($text);
          return new FilterProcessResult($converted_text);
        }
        catch (CommonMarkException $e) {
          $this->getLogger('my_module')->critical('Unable to convert markdown into HTML.');
    
          return new FilterProcessResult($text);
        }
      }
    
      /**
       * Generate an environment with extensions.
       */
      private function createEnvironment(): Environment {
        $environment = new Environment($this->config);
    
        // Add CommonMarkCoreExtension for commonly used parsers and
        // renders.
        $environment->addExtension(new CommonMarkCoreExtension());
    
        // Add additional extensions.
        // @see https://commonmark.thephpleague.com/2.4/basic-usage/#using-extensions
        $environment->addExtension(new StrikethroughExtension());
    
        return $environment;
      }
    
    }
    
  • πŸ‡­πŸ‡ΊHungary tikaszvince

    I've just attached an updated version of my patch from #5 to handle error on Drupal10.1 + PHP8.1 caused by INF passed as max_nesting_level.

    ---

    Hey @markdorison,

    Is there any news around how we can upgrade to Commonmark v2.4+?

  • Status changed to Needs review about 1 year ago
  • Open in Jenkins β†’ Open on Drupal.org β†’
    Core: 9.5.x + Environment: PHP 8.0 & MySQL 5.7
    last update about 1 year ago
    Build Successful
  • Open in Jenkins β†’ Open on Drupal.org β†’
    Core: 10.1.x + Environment: PHP 8.2 & MySQL 8 (--ignore-platfrom-reqs)
    last update 12 months ago
    Build Successful
  • Open in Jenkins β†’ Open on Drupal.org β†’
    Core: 10.1.x + Environment: PHP 8.2 & MySQL 8
    last update 12 months ago
    Build Successful
  • πŸ‡ΊπŸ‡ΈUnited States bsnodgrass

    There is are new comments in the issue Adopt CommonMark spec for Markdown files β†’ since the last comment here.

    The description on the Markdown Module states "There is current an issue open to make the CommonMark Spec the "official" Drupal Coding Standard." which may also need an update.

  • Status changed to RTBC 21 days ago
  • πŸ‡¬πŸ‡§United Kingdom joachim

    Tested the patch and it's working well.

  • πŸ‡³πŸ‡ΏNew Zealand jonathan_hunt

    I applied patch #14 on 3.0.1 and updated league/commonmark to 2.5.3, but unfortunately I immediately get a fatal error on any site page: Fatal error: Could not check compatibility between League\CommonMark\CommonMarkConverter::getEnvironment(): League\CommonMark\Environment\Environment and League\CommonMark\MarkdownConverter::getEnvironment(): League\CommonMark\Environment\EnvironmentInterface, because class League\CommonMark\Environment\Environment is not available in /app/vendor/league/commonmark/src/CommonMarkConverter.php on line 40

Production build 0.71.5 2024