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

Fix HTML escape issues #484

Closed chenzhiguang closed 1 year ago

chenzhiguang commented 1 year ago

This PR introduces a new syntax EscapeHtmlSyntax, fixes most of the HTML escape issues.

Benchmark

Before

Screenshot 2022-11-08 at 19 17 15

After

Screenshot 2022-11-09 at 05 00 19

coveralls commented 1 year ago

Pull Request Test Coverage Report for Build 3433754275


Files with Coverage Reduction New Missed Lines %
lib/src/inline_syntaxes/text_syntax.dart 1 85.71%
<!-- Total: 1 -->
Totals Coverage Status
Change from base Build 3430771645: -0.09%
Covered Lines: 1194
Relevant Lines: 1259

💛 - Coveralls
chenzhiguang commented 1 year ago

We might need to update flutter_markdown test cases.

kevmoo commented 1 year ago

@chenzhiguang – let's get your other PR landed and then I can look at flutter-markdown

OR you could look at https://github.com/flutter/packages/pull/2777 and take inspiration.

What I did: synced down the package repo locally and just hacked with a dependency_override on my local pkg:markdown until tests passed with and without the override.

kevmoo commented 1 year ago

@chenzhiguang – if the change IS related to fixing HTML escape, it's fine to leave, too!

chenzhiguang commented 1 year ago

@chenzhiguang – if the change IS related to fixing HTML escape, it's fine to leave, too!

It is not related, I will create some performance improvement PRs later.

chenzhiguang commented 1 year ago

What I did: synced down the package repo locally and just hacked with a dependency_override on my local pkg:markdown until tests passed with and without the override.

Thanks, I will do it when this PR is approved.

chenzhiguang commented 1 year ago

Cool, thanks. I will update flutter_markdown later.

srawlins commented 1 year ago

@devoncarew @kevmoo I want to cause the least chaos this time around. Must we / can we fix flutter_markdown before landing this?

kevmoo commented 1 year ago

We get fixes into flutter_markdown, land those, rerun the test here, then land this PR

On Thu, Nov 10, 2022 at 8:21 AM Sam Rawlins @.***> wrote:

@devoncarew https://github.com/devoncarew @kevmoo https://github.com/kevmoo I want to cause the least chaos this time around. Must we / can we fix flutter_markdown before landing this?

— Reply to this email directly, view it on GitHub https://github.com/dart-lang/markdown/pull/484#issuecomment-1310548536, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAEFCU2YDSORTQIDXANG43WHUOJ7ANCNFSM6AAAAAAR2TRRUU . You are receiving this because you were mentioned.Message ID: @.***>

chenzhiguang commented 1 year ago

The PR to fix flutter_markdown test cases: https://github.com/flutter/packages/pull/2797