- π¬π§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 12:31pm 9 June 2023 - 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 callmerge()
.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 11:24am 18 June 2023 - π©πͺ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
over 1 year ago 3:31pm 11 September 2023 - last update
over 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
about 1 year ago Build Successful - last update
about 1 year 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
about 2 months ago 11:58am 25 October 2024 - π¬π§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 - First commit to issue fork.
- π¬π§United Kingdom malcomio
Have created MR 39 based on patch #14
@jonathan_hunt I'm unable to reproduce the fatal error you mention, although in our installation we have patches applied for the following issues:
#3226069: Allow "incompatible" filters to be enabled (but validate that they appear after Markdown) β
π After each cache clear, visiting a page filtered with markdown causes calls to pecl.php.net; it is not clear why we are doing this Active
β¨ Add support for Commonmark v2 Active
π Subformstate incorrect interface error RTBC
π Error when saving text format - configuration property id doesn't exist Active
π Unable to save text format without enabling Markdown filter Active