dart-lang / markdown

A Dart markdown library
https://pub.dev/packages/markdown
BSD 3-Clause "New" or "Revised" License
443 stars 201 forks source link

Improve ease of use [proposal] #490

Closed chenzhiguang closed 1 year ago

chenzhiguang commented 1 year ago

Usage example of the new implementation

import 'package:markdown/markdown.dart';

void main() {
  const text = '# Hello "~~Markdown~~"!';

  Markdown( enableAtxHeading: false).parse(text).toHtml();
  // Output: <p># Hello &quot;~~Markdown~~&quot;!</p>

  Markdown.commonMark(escapeHtml: false).parse(text).toHtml();
  // Output: <h1>Hello "~~Markdown~~"!</h1>

  Markdown.gitHubFlavored().parse(text).toHtml();
  // Output: <h1>Hello &quot;<del>Markdown</del>&quot;!</h1>

  Markdown.gitHubWeb().parse(text).toHtml();
  // Output: <h1 id="hello-markdown">Hello &quot;<del>Markdown</del>&quot;!</h1>

  Markdown.gitHubFlavored(escapeHtml: false).parseInline(text).toHtml();
  // Output: # Hello "<del>Markdown</del>"!

  Markdown(
    extensions: [HighlightSyntax()],
  ).parse('==Markdown==').toHtml();
  // output: <p><mark>Markdown</mark></p>
}

class HighlightSyntax extends DelimiterSyntax {
  HighlightSyntax()
      : super('=+',
            requiresDelimiterRun: true,
            allowIntraWord: true,
            tags: [DelimiterTag('mark', 2)]);
}

Major change

1. Deprecate Document, use Markdown instead

With the current Document implementation, if a user wants to have only some syntaxes enabled, the only way to achieve that is set both withDefaultInlineSyntaxes and withDefaultBlockSyntaxes to false and pass in the desired syntaxes through blockSyntaxes and inlineSyntaxes, it is tedious also easy to make mistakes, for example the order of syntaxes matters, you can not put InlineHtmlSyntax after EscapeHtmlSyntax, ParagraphSyntax must be the last block syntax etc.

2. Deprecate markdownToHtml(), use List.toHtml() instead.

We have to maintain these tens of parameters which will be passed to Markdown class if we keep the markdownToHtml() function, also List<Node>.toHtml() is more elegant.

3. Deprecate ExtensionSet, use Markdown factory constructors instead.

4. blockSyntaxes and inlineSyntaxes options of Document class vs extensions option of Markdown class.

They do not make a big difference, just make it simpler to pass in custom syntaxes.

coveralls commented 1 year ago

Pull Request Test Coverage Report for Build 3446557993


Changes Missing Coverage Covered Lines Changed/Added Lines %
lib/src/markdown.dart 53 68 77.94%
<!-- Total: 117 132 88.64% -->
Totals Coverage Status
Change from base Build 3446153575: -0.7%
Covered Lines: 1286
Relevant Lines: 1366

💛 - Coveralls
chenzhiguang commented 1 year ago

This proposal is not a prototype, it is fully functional.

chenzhiguang commented 1 year ago

Can we have a look at it? I believe it is a good one.

jonasfj commented 1 year ago

This is actually pretty cool. But I would suggest doing it in smaller steps.

Like:

  1. Deprecate markdownToHtml(), use List.toHtml() instead.

That could be a PR on it's own. And it's much easier to get a review if it's not mixed with many other changes.

jonasfj commented 1 year ago

4. blockSyntaxes and inlineSyntaxes options of Document class vs extensions option of Markdown class.

I think this is an interesting idea. How far down this rabbit hole would we want to go.

At the extreme end we could make all the BlockSyntax and InlineSyntax implementations private, and completely forbid injection of custom syntaxes. It would make the package less flexible, but perhaps limit the number of bugs.

Like, right now the ordering of various syntaxes might affect how something is parsed. Or am I wrong?

chenzhiguang commented 1 year ago

Thank you for reviewing. Yes, this proposal generally demonstrates the idea and proof of concept. We should definitely break it into smaller pull requests.