parsedown
parsedown copied to clipboard
erusev
Headerless table
A different take on headerless tables where syntax omits the header instead of leaving it empty – inspired by #538.
~~This also includes a bugfix commit to permit single column tables without need to include pipe delimiters in the header and rows (delimiter row containing pipe delimiters is enough).~~
If merged, closes #538.
I dropped out the bugfix commit and have merged it ahead of this PR via #584 since I found a related issue open.
Copy pasting the pros and cons list specific to this implementation from https://github.com/erusev/parsedown/pull/538#issuecomment-376339414 since it probably merits inclusion in the actual PR:
A couple things to note about
| --- | --- |
| abc | def |
Pros:
- No need to write empty structure delimiters
- Very analogous to an already supported syntax in GitHub flavoured markdown for tables with no body:
| abc | def | | --- | --- | - Syntax is currently an invalid table in GitHub favoured markdown, so will not collide with any sort of already valid table
Cons:
- The failure mode for text that uses a different parser is likely to be no table rendering at all (whereas the other variant proposed here would render a table with empty whitespace in the header as a fallback).
- No one currently seems to support this variation: Babelmark2
- Deviates from GitHub flavoured markdown spec
I proposed this as a syntax to the GitHub flavoured markdown spec (https://github.com/github/cmark/issues/91), though the repo seems not to be as active as it once was.
I think this behaviour would be a good fit for inclusion in the GFM spec as it is very analogous to the spec's way of omitting a table body, and doesn't really add more complexity IMO (both in the parser and human readability).
That said, unfortunately I'm not sure if there will be much activity in the GFM repo in the near future for discussions: https://github.com/github/cmark/issues/88#issuecomment-367512275
[...] I'd happily take a PR that adds this. (My work focus has shifted onto other priorities, unfortunately.)
Hence, I'm giving some thought as to whether it would be sensible to merge this but gate it on the upcoming strict mode being off (since it isn't spec behaviour). i.e. this would be default, unless you opt-in to stricter behaviour.
Maybe we should think about how to deal with spec differences and breaking changes in Parsedown in general. As far as I can tell we try to reconcile the following specs/goals/requirements with one another:
- CommonMark
- GitHub flavored Markdown
- John Gruber's original Markdown specs
- Markdown Extra
- Things that most Markdown parsers do without written specs ("de facto" standards)
- Maintaining backwards compatibility to older Parsedown versions
- Custom spec extensions (like this headerless table syntax)
We should furthermore keep in mind that CommonMark has some sort of "claim to exclusivity": By adding non-CommonMark syntax rules and behaviours we violate the CommonMark specs. We will never be fully compatible to CommonMark, but we should at least try our best. I think the main Parsedown parser should aim for CommonMark compatibility.
I think the best way to deal with this situation is to introduce a way more flexible plugin infrastructure. Extending the Parsedown class is rather a workaround than a real solution. Extensions shouldn't extend the Parsedown class. They should rather be able to hook into Parsedown's processing, allowign one to change Parsedown's behavior at certain, pre-defined points. This allows one to load multiple plugins at the same time, allowing us to move things like this headerless table syntax to separate plugins that focus exactly one feature. Since Parsedown is a rather small and manageable project we shouldn't introduce a complex plugin system, Hooking and passing certain variables by reference to plugins using a ParsedownPluginInterface::handleEvent(string $eventName, array $parameters) method should do the job.
Since Parsedown is a rather small and manageable project we shouldn't introduce a complex plugin system, Hooking and passing certain variables by reference to plugins using a
ParsedownPluginInterface::handleEvent(string $eventName, array $parameters)method should do the job.
I'd be up for something like this. In-fact, sorting out a sustainable way of making plugins is something I'd really like to do sooner rather than later.
I'm a little nervous about the impact of releasing 1.8.0 as it stands: even though there are literally no breaking changes in the public API, extensions are inevitably going to break because they have hooked into observed internal behaviour and now rely on it. For example, slightly different element structures will be returned by Parsedown's core block handlers creating potential type mismatches in places in extensions: mentioned in passing here.
I've implemented a couple of compatibility layers for converting old looking blocks, inlines, and handlers to the more AST friendly methods. But there are some things that just aren't fixable in this way or won't be known because extensions could literally be relying on anything.
I don't really think there is going to be a way avoiding issues with extensions if the repo is to move forward (bar cutting a new major version). If we could get something better set up then we could at least avoid this situation again so extensions only have to migrate once.
Maybe it's better to release a new major version and explicitly breaking BC, than adding more and more complexity just to maintain a very fragile compatibiliy to old extensions. There's even a risk that we might break BC in a non-obvious way, likely resulting in confusion and frustration. I'm not sure how common the use of Parsedown extensions is in general, and how common it is to use third-party extensions (besides Parsedown Extra). I would assume that e.g. many CMS' that use Parsedown have their own extension and know how to upgrade their extension as we publish a new major release.
Anyway, virtually the only hard thing about implementing a plugin infrastructure based on Hooking is to identify where the hooks should be and what variables should be passed.
adding more and more complexity just to maintain a very fragile compatibiliy to old extensions.
Just for visibility, these are the compatibility layers I'm talking about (they're effectively equivalent to API deprecations since they just map over old ways of doing things).
The following rewrites the markup block/inline key to rawHtml so we can put raw HTML in the AST (instead of it being collected in a string by the line/lines methods):
https://github.com/erusev/parsedown/blob/0a842fb5b1a9a56428e5ea527b7bf04dda6ce36d/Parsedown.php#L327-L335
The second converts the old handler strings to the more verbose handler arrays that allow selecting a destination like elements, or text (old handlers effectively mapped straight to HTML).
https://github.com/erusev/parsedown/blob/0a842fb5b1a9a56428e5ea527b7bf04dda6ce36d/Parsedown.php#L1594-L1600
Maybe it's better to release a new major version and explicitly breaking BC
Drawing a line would be nice for a few reasons (for example I'd really like to namespace and split up Parsedown a little into a few files so we can make better guarantees with PHPs type system about the structure of some of our (currently informal) types like blocks, elements, and inlines. We could still offer a release Parsedown.phar for anyone that wants to include a file instead of the composer installation).
But anyway, this is for another thread.
A new major version would be really nice I guess is my point – I suppose the question is whether or not we should release one now just to avoid breaking extensions. As it currently stands: nothing relying on public API behaviour will break AFAIK – in-fact there are a ton of bugfixes lined up, so users just using Parsedown will likely see it run better.
I'm not sure how common the use of Parsedown extensions is in general, and how common it is to use third-party extensions (besides Parsedown Extra).
Grav in particular seems to be home to a few extensions (some third party): see https://github.com/getgrav/grav/pull/1972
That said, there haven't been a huge number of complaints about 1.7 (which was non public API breaking, but did break some extensions), so maybe the majority of users do not use extensions 🤷♂️.
(For what it's worth: Parsedown-extra is already compatible with the proposed Parsedown 1.8.0 in master).
I'de definitely vote for a cleaner version of Parsedown with all the AST improvements as a 2.0 release.
We (Grav CMS) are pretty locked into Parsedown moving forward and we did experience a few issues with 1.7 and had to patch our releases a couple of times after Parsedown was updated and it got picked up in our builds. In fact, in the end, we rolled back to Parsedown 1.6.4 in Grav 1.4.3 because there were too many breaking issues, and while Parsedown Extra is compatible with 1.8, it's pretty broken with 1.7.
As mentioned, we also have Parsedown extensions that break badly with 1.8 (Exceptions that break any Grav site with those extensions installed), I originally hoped to find a graceful solution, but now i'm leaning towards holding off on 1.8 completely until we can release Grav 2.0 where breaking changes are more acceptable.
Per semantic versioning, Parsedown 2.0 would give you the opportunity to break compatibility without any worries, and remove all the fallback stuff, and focus on getting the codebase clean and optimized.
Right then, let's go for 2.0. https://github.com/erusev/parsedown/tree/2.0.x
@aidantwoods Now it has been 2,5 years since you all started working on 2.0 and I wanted to ask if there is anything one can do to anticipate the process? 🙂
