Merge in the CommonMark project

I 100% support this.

I've gone ahead and created #2952531: Transfer ownership of Markdown [filter] module.

Once that is done, we can move forward here.

I have actually made some recent progress in some other modules I maintain/co-maintain that may help with this issue as well:

https://www.drupal.org/project/any
https://www.drupal.org/project/backport
https://www.drupal.org/project/backport_composer
https://www.drupal.org/project/composer_manager

The idea being we could probably just create 8.x code and then support a 7.x-2.x branch using the same code.

This would allow us to not have to worry about how the league/commonmark PHP library is added to a site (i.e. how composer is utilized).

Actually, going to mark this as postponed until #2952531: Transfer ownership of Markdown [filter] module is resolved.

This is no longer postponed.

I will try to do this something this week(end).

I've committed some initial refactoring work, but it's mostly just based on D8 APIs (plugins). I haven't actually tested any of this in a browser yet.

Do we really need an upgrade path? This is going to be vastly different from prior implementations. Not even sure what should be upgrade exactly. Thoughts?

Not specifically. The 7.x version is just going to be an exact copy of the 8.x code using #2954124: Add support for Filter APIs.

I was more specifically talking about any config/settings/variables. There were only a couple, so not entirely sure its worth it.

Ah, it's just 8.x-1.x that introduced the settings/config:
http://cgit.drupalcode.org/markdown/tree/config/install/markdown.setting...

And selector:
http://cgit.drupalcode.org/markdown/tree/src/Plugin/Filter/Markdown.php#n48

Yeah... I'm tempted to say that the "upgrade path" for this should just be to tell people to reconfigure their formats/filters (since there likely isn't that many).

The actual markdown filter machine name hasn't changed so content shouldn't be affected.

https://markdown.unicorn.fail is now live.

P.S. It may be beneficial in the future to come up with a more semantic/dedicated domain, but for now, this works.

Gone ahead and created a 8.x-2.0-alpha1 release for the work so far.

This doesn't actually expose any of the settings UI yet (which still needs work), but it at least provides working parser(s).

Weekly Meeting
Every Thur 4:00 PM CDT (UTC-05:00)
Hangout: https://tinyurl.com/yblctybm
Google calendar: https://tinyurl.com/ybnvlgen
iCal: https://tinyurl.com/y9sy9oem

Going to start this just to get it on the table. Whether or not there will be much to discuss is another thing.

It's an offset, not actual UTC time.

I suppose it's actually 4:00 PM CDT (UTC-05:00). I forgot that we're in daylight savings time here.

https://en.m.wikipedia.org/wiki/UTC%E2%88%9206:00

Just subscribe to the calendar. That should clear up any confusion.

Well. No one showed up for this week's meeting...

@markcarver, Do you still need help getting this to a beta or full 2.x release?

Some help would be nice. There are only a few remaining tasks:

• Build each parser's settings and relevant extension settings
• Add support for optional extensions (table/attributes)
• Make sure everything works (add tests)
• Add/fix documentation

Hi Mark - I have a bit of time to help you out if you need / want.

Can see you have a bit of momentum going now but let me know if you need assistance - personally, beyond what you are doing with the GFM environment and the inbuilt extensions, I am keen to also get the footnotes extension working (https://github.com/rezozero/commonmark-ext-footnotes) - perhaps I could help getting that in there as that would be a good example of an optional extension? Or would you prefer I wait until you have got the last bit of the extension settings done?

Please, feel free to create new issues as you see fit. I will welcome any and all help :D Just keep in mind that I'm still rearranging some things, so be prepared to rebase/reroll your patches.

edit: It will be easier to merge new/additive code and I will attempt to do that often prior to any more of my work.

Nice this is rolling. Rock on!

Ok, sounds good - I'll just start with making a footnotes issue. Also happy to help with documentation when you have locked things down a bit more, let me know.

Oh and I see tests are on the list and don't exist yet - I could take a crack at starting on something there.

Small patch that fixes the version working.

+++ b/src/Plugin/Markdown/Parsedown.php
@@ -40,11 +40,13 @@ class Parsedown extends BaseParser implements MarkdownParserBenchmarkInterface {
+
+    return NULL;

We don't have to explicitly return NULL here.

---

You can create new issues for specific patches. This issue is primarily a catch-all for me specifically as I don't know I'll be touching until I do (and often lumps a bunch of changes together in a single commit).

Also, at some point, we're going to need to re-test all this with https://www.drupal.org/project/backport for 7.x support.

Ahh yep, think just needed to remove the constraint of returning a string. I'll create specific issues and patches from now on 👍

#53: very nice to see this moving forward again! 🥳

#57: yay, thanks for pushing footnotes forward! Definitely missing that right now.

Okie dokie. I know #62 is a lot, but honestly it's really necessary. I'll try to break down what's going on and why:

• Removed hard dependency on league/commonmark (it's now just a composer suggestion and also the lowest plugin weight so it's selected first on install if multiple parsers are detected).
• The module will not install if there are no parsers detected (see markdown.install which has been complely overhauled). Note: there is a massive and ugly regression in Drush 9+ that doesn't actually allow this to work properly though: https://github.com/drush-ops/drush/pull/4337
• Lowered PHP version limit to 5.5.9 (Drupal 8.0.0 minimum). This allows parsers that don't require higher PHP versions like Parsedown to be used on sites where this may be an issue.
• The main module was doing way too much and is now split out into separate sub-modules: markdown_benchmark, markdown_filter, markdown_twig. This allows sites to only enable need certain functionality that they actually need. The main module will serve as the primary APIs and services for interacting with parsers and extensions.
• New "global" Markdown settings form located at /admin/config/content/markdown. Previous code made this functionality really wonky and unreliable unless it was tied to a filter (that shouldn't be the case). The filter's settings will eventually extend from this form to save code.
• New administer markdown permission added for ^.
• New schema mapping added for proper parser and extension settings config export.
• I went ahead and created a #markdown channel on slack.

Still todo in regards to the above (which I plan on finishing up tomorrow):

• Finalize markdown_filter (haven't focused on it since I split it out, so not sure what is or isn't working there)
• Cleanup the rest of the code so it isn't referencing any "filter" parameters or configuration. If the markdown_filter module is used, it should pass along the configuration to the parser. The parser doesn't need to know about the filter.
• Specifically cleanup \Drupal\markdown\Markdown so it doesn't have long winded parameters in methods like $parser = NULL, $filter = NULL, AccountInterface $account = NULL, LanguageInterface $language = NULL.
• Maybe some initial/rudamentary tests.
• Likely other stuff I'm not remembering at the moment, but I'll probably get to most of it.

edit: oh and I've also been slowing working on getting https://markdown.unicorn.fail back up and running

Impressive! Once it's beta enough, i can push it to our internal ticketforum for live beta.

Phew. Ok. I believe the majority of the parser + extension APIs are relatively "stable" now.

Two new CommonMark extensions were implemented (need to flush out extension settings and dependencies):
#3131218: CommonMark Extension: Heading Permalinks
#3131220: CommonMark Extension: Table of Contents

The UI has been completely redone using vertical tabs and summaries.

Every parser now also has "Allowed HTML tags" settings which is really just a slightly customized FilterHtml implementation to allow more permissive global attributes.

Still need to do a little clean up on a few things:

• CommonMark setting "HTML Input" needs to add states to the "Allowed HTML tags" to hide it and not use it when options other than "allow" is set
• #3129847: Add support for CommonMark extensions
• Step through and fix markdown_benchmark and get https://markdown.unicorn.fail up and running again
• Documentation cleanup

We're close to a stable release! I'm thinking that once the above is done, we can release an RC and then in a month maybe a stable?

(edit) p.s. I still may rename/move some classes/namespaces around. Would like to organize it a tad better, but... naming things--

Also, I would say that we're at a good stage for people to start reviewing.

What's the best way to review the changes? Just look everything on the 8.x-2.x branch?

Yeah, that would probably be the best. Alternatively, you could browse the commit history to see how much I went back and forth :D

Although, it may be better to just install it from scratch and look at it via the UI and filters, etc. See if any "jumps out".

Here are some screenshots for those who are interested but don't have an install ATM:

Parser:

Heading Permalink:

Table of Contents:

#63: thanks for all that progress, and thanks for the thoughtful overview! Just one question: isn't the main purpose always filtering? Why have the filter plugin live in a separate markdown_filter module?

#74: wow, thanks for sharing those (pretty!) screenshots! I get the sense you like strict separation of concerns, strict adherence to best practices and strict validation. If that hunch is right, I think you'll like the approach I took in the CDN module with regards to ensuring strict validation of configuration: #2969065: Use typed config validation constraints for validation of cdn.settings simple config. It's basically where core is headed too, but which nobody has energy/time for. It's building on foundations that already exist in core for a long time now. It's got +1 from config system maintainers. I think you might want to do something similar prior to a stable release because it can give you much more confidence about how people use this module, and reduce the number of support requests/bug reports :)

Just one question: isn't the main purpose always filtering?

No, or rather not anymore. It's just the most common (end-user) use case. The reality is that being able to parse markdown shouldn't be tied to (dependent on) any Drupal "filter (text_format)". It should be able to be invoked programmatically in a standalone fashion.

I'm primarily thinking of #1674976: Allow README.md to optionally render as the project page where the use of a field/filter wouldn't really be necessary as it could simply parse the project's README.md file directly (see Markdown::loadPath(), method for loading/caching markdown files based on mtime).

Another use case I know of personally is https://drupal-bootstrap.org where the project markdown files are parsed in API hooks and would be even more burdened if it had to process everything through some custom filter (text_format) which requires setting up an actual working text_format, complete with its own config, filters, roles, etc.

Why have the filter plugin live in a separate markdown_filter module?

So, I did this initially because I was attempting to separate concerns and decouple all the "filter" stuff from the main module... for reasons ^.

Now that it's sufficiently decoupled and it's maintained more or less a relatively small size, I can seem moving it back into the main module again. Perhaps also the markdown_twig sub-module as well. This was my "p.s." remark at the end of #70 about renaming/moving some classes/namespaces around.

The markdown_benchmarks module should definitely remain separate as it's only intended for specific use cases.

I get the sense you like strict separation of concerns, strict adherence to best practices and strict validation.

I've definitely been leaning more towards that direction over the years. It just makes things very clear on what they do/intend to do.

I've been able to get the config schema working the way I want it:
https://git.drupalcode.org/project/markdown/-/blob/26fdcbfdcca58961d265d...

As far as constraints and validations, that's certainly something we can add in down the road, but I don't think it's necessarily a release blocker.

If it means that implementing it will break something and cause BC issues, then we can just bump the major again.

For now, I think the config types should be sufficient given that there isn't really a whole lot of validations that need to take place currently. Most settings are boolean/select based; a few are text fields here and there.

Given that the parsers themselves typically validate their settings or, at the very least, would likely just ignore it if something was awry, I think we can get away with exceptions being thrown in the unlikely event something was completely borked.

I'd rather not delay what has already taken a long time, especially given that it appears that it's dependent on my available time.

@markcarver Awesome work! It shows how much work you've put into this.

OK.

Last "refactor", I swear lol

Sorry, the majority of this one was to make way for the introduction of Render Strategy:

So this is two-fold:

1. Intentionally separates the XSS handling from the parsers. If a parser has settings relevant to this topic and there's a [Drupal] render strategy being used, it disables those settings so they're not in conflict. This is, primarily, to assist users who may not understand how all this is supposed to work together; in harmony.
2. Introduces as new @MarkdownAllowedHtml plugin that can literally be used anywhere: modules, themes, filters, parsers, extensions... where ever. I was partially inspired by #3133121: Support embedded media and realized that knowing exactly what should be allowed through an XSS filter can be extremely complex. This is an attempt to simplify (as much as possible) that complexity. This also means that the default "Allowed HTML" is a much shorter list as the idea is to enable only what's needed as it's needed.

Some other noteworthy mentions:

• Moved the filter and twig extensions back into the main module.
• Moved/renamed a lot of classes so they're not unnecessarily prefixed and just lets the namespaces to the work (doh!)
• Finished filling out the Parsedown and PHP Markdown library settings.
• Introduced a new feature that allows you to reset to the default value on any setting by clicking the ↩️ at the end of the label when its overridden.
• Copied some of core's newer classes into src/BcSupport so it can be installed on Drupal 8.0.0 (surprisingly didn't need that that many) and have actually been testing it on a local 8.0.0 install.
• Removed "guides" in favor of a separate issue #3135355: Implement "Examples" plugin manager
• Removed markdown_benchmark in favor of a separate issue #3135364: Add markdown_benchmark back
• #3135360: [markdown] Add/fix documentation (also created https://www.drupal.org/docs/8/modules/markdown)
• #3135359: [markdown] Add Tests

With all this being said, I'm closing this issue as "Fixed".

If you find any issues or bugs, please just create a new issue so we can better keep track of things now.

Thanks Mark!

> Render Strategy

Wow! A module to rule them all.

Now that #3129847: Add support for CommonMark extensions is done, I've gone ahead and created 8.x-2.0-rc1.

If y'all can test it out this week, that'd be great. Also, if anyone feels like tackling some documentation, that would be helpful too.

If no major issues creep up in the next week or so, my plan is to push a stable 8.x-2.0 release this next weekend (5/16).

#76:

I'm primarily thinking of #1674976: Allow README.md to optionally render as the project page where the use of a field/filter wouldn't really be necessary

Makes sense, thanks for pointing that out! :)

Now that it's sufficiently decoupled and it's maintained more or less a relatively small size, I can seem moving it back into the main module again.

+1

The markdown_benchmarks module should definitely remain separate as it's only intended for specific use cases.

+1

As far as constraints and validations, that's certainly something we can add in down the road, but I don't think it's necessarily a release blocker.

+1

Given that the parsers themselves typically validate their settings or, at the very least, would likely just ignore it if something was awry, I think we can get away with exceptions being thrown in the unlikely event something was completely borked.

+1

#80: once again epic work!

Now i found some time to update this module on a quite heavy project. Some notes on this:

* Doing drush updb with RC1 sent me into the hard to spot #3136267: Error: Unsupported operand types in SettingsForm.php on all updates that messed with user roles
* Using todays dev #df3e4119703 sent me into #3138551: Fatal error: Interface 'League\CommonMark\Extension\ExtensionInterface' not found but composer require league/commonmarkmade the update run then

More notes to come.

I am having difficulty following this issue. Markdown 8.x-2.0-alpha1 installed on Drupal core 8.8.4. Now updating to Drupal 8.9.6. What steps should I take to be able to use Markdown in the Drupal 8.9.6 project? Do I have to install league/commonmark? Any help would be much appreciated.

#87: Yes, you need to also require league/commonmark to use CommonMark with Markdown 2.x. There's actually a 8.x-2.0-rc1 release, which likely has a bunch of improvements over the alpha.

Thank you, @Ambient.Impact!