markdown
markdown copied to clipboard
dart-lang
Implement `AlertBlockSyntax` using `BlockquoteSyntax`
A proposed fix for https://github.com/dart-lang/markdown/issues/584
As I understand the alert-block syntax, it is an extension on the original blockquote support in markdown. Basically, it's a blockquote that is rendered differently.
That means that if alert-block syntax isn't supported a markdown parser will render it as a blockquote. This provides a nice graceful fallback. It also means that if you're implementing it, you really should implement it the same way you implement blockquote parsing.
This is an attempt at implementing AlertBlockSyntax using the same parsing logic as BlockquoteSyntax.
This is not pretty. Ideally, whether to support alert-syntax should be a boolean option passed to BlockquoteSyntax. Because an alert-syntax is a blockquote with extra bling. But that's not how this package is structured :rofl:
In fact, it can probably be argued that trying to make a markdown parser that consists of composeable objects is extremely sketchy. Especially, when we let consumers of the package implement their own syntaxes and inject them into the whole mix. It's very hard to determine how this whole thing works (and what combinations of syntaxes will work). It would probably have been safer to pass in all the option as boolean configuration options that enable/disable a particular syntaxes in a single large parser.
Currently, running crash-test on this, though that won't reveal if we have bugs. Just that it doesn't crash :rofl:
Pull Request Test Coverage Report for Build 8007970317
Details
- -1 of 18 (94.44%) changed or added relevant lines in 1 file are covered.
- No unchanged relevant lines lost coverage.
- Overall coverage increased (+0.2%) to 96.345%
| Changes Missing Coverage | Covered Lines | Changed/Added Lines | % |
|---|---|---|---|
| lib/src/block_syntaxes/blockquote_syntax.dart | 17 | 18 | 94.44% |
| <!-- | Total: | 17 | 18 |
| Totals | |
|---|---|
| Change from base Build 7977582785: | 0.2% |
| Covered Lines: | 1529 |
| Relevant Lines: | 1587 |
💛 - Coveralls
No crashes scanning all markdown files in the latest version for all packages on pub.dev
Scanned 49735 / 49913 (skipped 0), found 0 issues
## Finished scanning
Scanned 49913 package versions in 0:23:20.241887
23:21 +1: All tests passed!
I've just landed https://github.com/dart-lang/markdown/pull/593 as a fix for the crash, but I think this PR is probably going in a direction that would be good in the long term, so I'd like to keep it open. I'll try to review more tomorrow.
I looked at it today, and the only think I don't like is the same thing you don't like @jonasfj haha, which you elaborate on above.
I think there should be a way to "nicely" sit this syntax into the GFM extension, "on top" of the blockquote syntax.
In fact, it can probably be argued that trying to make a markdown parser that consists of composeable objects is extremely sketchy.
Haha this is really true. It's a weird concept, to have a plugable parser like this, based on an "ordering" of syntaxes, where we ask each in an order, "Can you parse this line? No? And you?" It's inherently flawed.
Haha this is really true. It's a weird concept, to have a plugable parser like this, based on an "ordering" of syntaxes, where we ask each in an order, "Can you parse this line? No? And you?" It's inherently flawed.
It's completely orthogonal, maybe it deserves a controversial proposal, where we can shoot down the idea :D
Filed https://github.com/dart-lang/markdown/issues/594