Come together with the global Drupal community in Rotterdam, 28 Sept – 1 Oct 2026. Sessions, contribution, connection, and Early Bird savings until 8 June.Integrating with Libraries API will make it easier to upgrade the markdown library, as well as preventing issues like #684554: Creates invalid HTML with preformatted blocks..
| Comment | File | Size | Author |
|---|---|---|---|
| #8 | 1532380-markdown-integrate-with-libraries-api-8.patch | 86.55 KB | barraponto |
| #1 | 1532380-markdown-integrate-with-libraries-api.patch | 86.22 KB | barraponto |
Comments
Comment #1
barrapontoComment #2
barrapontoTagging.
Comment #3
RobW commentedAside from bringing the module in line with Drupal external library best practices, these issues could all be fixed or eased with Library API integration:
#1439150: Use new version of PHP Markdown library because the old version is not compatible with PHP 5.3
#1507606: MultiMarkdown/Markdown Extra
#1413386: Update to PHP Markdown Extra 1.2.5 which was release on January 8, 2012.
#684554: Creates invalid HTML with preformatted blocks.
Marking this as major and testing the patch. If others agree, I think we should mark the above issues postponed as well.
Thanks barraponto!
Comment #4
barrapontoOne thing to note: adding a couple of dependencies (the library module and the actual markdown library) means we should do it in a major release (7.x-2.x) to make sure people read the upgrade notices. Some other UI/String changes like #1319492: Disable certain parts of Markdown and #1148542: Prevent buggy output caused by input filter order would benefit from a major release as well.
Comment #5
barraponto@robw: by the way, applying this patch would mean marking all of the other as duplicates, since they'd befixed by an upgrade.
Comment #6
RobW commentedRight, I mean postpone those issues now so that users who are watching them know to come here to work on a solution.
Comment #7
sreynen commentedThis is a good idea, but I think the implementation needs some work. If we're going to add calls to the Libraries API module, it should be added as a dependency in the .info file, not just suggested as an install in the README. If someone skipped the README, this would start throwing fatal errors causing WSOD. And pretty much everyone skips the README on an update.
Comment #8
barrapontoYeah, libraries should be a dependency.
Comment #9
frjo commentedI will work on making use of Libraries API 2 to load a Markdown library. This work will be done in a new 7.x-2.x branch.
I will try to make it possible to choose between Sundown, Multimarkdown, PHP Markdown, PHP Markdown extra etc.
Drush commands to install the different Markdown libraries would be nice as well.
Comment #10
barraponto@frjo this patch is a good start for a 7.x-2.x branch. Adding support for different markdown libraries make it easier to swap them later (or support multiple libraries, as WYSIWYG module does). I've been using this patch on a production site for a while.
So, focusing on getting libraries support, this patch is NeedsReview. Support for multiple libraries is a followup issue, depending on this patch.
Comment #11
frjo commentedI'm sure the patch works but I want to implement the Libraries 2 API with hook_libraries_info() and libraries_load(). I have done this already for Colorbox 7.x-2.x.
Comment #12
frjo commentedA first stab at Libraries API 2.x implementation as well as a drush command to download the PHP Markdown plugin is now committed to 7.x-2.x. Please test it.
Comment #13
barraponto@frjo if you want us to test, post the patch and mark as needs review. wait for testers to set the status to reviewed and tested by the community and commit after RTBC.
See http://drupal.org/node/363367 for more on contributing best practices. I'll be glad to help you maintain markdown module (i'm a comaintainer as well).
Comment #14
frjo commented@barraponto, you are right of course. I think we can assume that people interested in Markdown know about patches as well :-).
I have had problem on other modules to get people to test if there was only patches.
Comment #15
Grayside commentedAlso #1587726: Support php-sundown.
It's unclear to me that supporting the different Markdown libraries is simply a Library API issue. Different libraries also have syntax extensions and so on. Is the idea that the Markdown module will adapt to the library you've selected? (E.g., filter tips, compatibility with other filters, etc)
Comment #16
barrapontoIf you use a library with extended syntax, as long as it is backwards compatible, you're fine. If you ever go back to regular markdown, then it's really an issue, but not for the module mantainers :P
However, new libraries might provide new options. Those options (and therefore those libraries) are not officially supported until we patch the module to detect the library and provide the configuration forms.
@Grayside If you feel that's the case with Sundown, please reopen that issue.
Comment #17
barrapontoAlso, since code is already commited, this issue is fixed already IMO.
Let's get this code out the door as a new version (alpha) and let the support requests and bug reports come.
Comment #18
Grayside commentedThis is the part I wanted to understand. It sounds like you want to head in the direction of Libraries API for the library, and some other mechanism for extended capability support as it comes up.
The ultimate progression of that strikes me as an inheritance-capable plugin mechanism. One plugin would define the tips and so on for core Markdown functionality, with inheriting plugins adding the extensions provided by, e.g., Markdown Extra. If that is a reasonable prediction, you get very close to "Filter module for wrangling full-syntax-stack external transformation libraries".
Comment #19
sreynen commentedMaybe you're right, but that's not my reading at all. And since your reading leads to vastly broadening the use case for this project, I hope you're misreading. I don't see any reason we'd need a plugin system for the relatively few Markdown processor options available in PHP. We can do this with a simple variable to track whether Markdown Extra (or Sundown, or ... is there even anything else?) is enabled, and some code to act differently based on the value of that variable.
I re-opened the Sundown issue, because that's not really a duplicate of this, as indicated by this being fixed while Sundown is not.
Comment #20
barrapontoLibraries API decouples the actual library (markdown, sundown, etc) from the module. Now, there are some particular integration cases that we might want to support by exposing it in the input filter configuration. It already supports markdown and markdown-extra features.
If you think it might be worth to actually make it pluggable, so that you can provide support in contributed modules, patches welcome. I don't think there are that many markdown implementations in PHP, so we might just run a switch statement and call it a day.
But anyway, we really need a use case. And code (in a new issue, it has nothing to do with Libraries API).