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

introduce a Line class #494

Closed chenzhiguang closed 1 year ago

chenzhiguang commented 1 year ago

The Line is used to maintain the tabRemaining for now.

This is a use case of Line, it can also explain the meaning of the tabRemaining: https://spec.commonmark.org/0.30/#example-6

Normally the > that begins a block quote may be followed optionally by a space, which is not considered part of the content. In the following case > is followed by a tab, which is treated as if it were expanded into three spaces. Since one of these spaces is considered part of the delimiter, foo is considered to be indented six spaces inside the block quote context, so we get an indented code block starting with two spaces.

In this example the tabRemaining is the two spaces from blockquote which will be used in the nested indented code block.

coveralls commented 1 year ago

Pull Request Test Coverage Report for Build 3577142422


Changes Missing Coverage Covered Lines Changed/Added Lines %
lib/src/block_parser.dart 4 5 80.0%
lib/src/document.dart 2 4 50.0%
lib/src/block_syntaxes/block_syntax.dart 1 5 20.0%
<!-- Total: 78 85 91.76% -->
Files with Coverage Reduction New Missed Lines %
lib/src/util.dart 1 98.18%
lib/src/block_syntaxes/fenced_code_block_syntax.dart 1 98.08%
<!-- Total: 2 -->
Totals Coverage Status
Change from base Build 3542193180: -0.3%
Covered Lines: 1230
Relevant Lines: 1300

💛 - Coveralls
chenzhiguang commented 1 year ago

Can we review this sooner? I need it to create PRs to fix the tab related issues.

kevmoo commented 1 year ago

We'll need to get this working w/ flutter_markdown – which is tricky because the API has changed!

chenzhiguang commented 1 year ago

Yes, I think we can use the same way as we did before to fix it.

chenzhiguang commented 1 year ago

Now it looks better.

kevmoo commented 1 year ago

Help me understand the point of Line? The API is now a bit more complex. What do we get by having a Line construct instead of just using String? (Might be good to have in the change log, too)

chenzhiguang commented 1 year ago

I found It seems the tab issues could be fixed without using this Line type. I will do more research tomorrow.

The idea came from https://github.com/tagnote-app/dart_markdown/blob/master/lib/src/line.dart, but the Line there maintains much more information.

kevmoo commented 1 year ago

Also: mind running benchmarks?

chenzhiguang commented 1 year ago

No difference, I have tried. Let me do it again

kevmoo commented 1 year ago

just because we're allocating a bit more...

chenzhiguang commented 1 year ago

Benchmark before

Screenshot 2022-11-16 at 21 59 12

Benchmark after

Screenshot 2022-11-16 at 21 58 29

chenzhiguang commented 1 year ago

We might need this Line in the future.

chenzhiguang commented 1 year ago

@kevmoo We might still need Line to maintain the tabRemaining, we can not just simply convert the tabRemaining to spaces and prepend them to a string, then passes it to the nested structure.

For example: (See: https://spec.commonmark.org/0.30/#example-5)

- foo

→→bar

There are 2 space sized tabRemaining from the list block, if we just prepend two spaces to →bar like this ..→bar, the output of the indented code block will be: (See: https://spec.commonmark.org/0.30/#example-2)

<pre><code>foo
</code></pre>

More information about tabs behaves: see https://spec.commonmark.org/0.30/#tabs

Tabs in lines are not expanded to spaces. However, in contexts where spaces help to define block structure, tabs behave as if they were replaced by spaces with a tab stop of 4 characters.

Thus, for example, a tab can be used instead of four spaces in an indented code block. (Note, however, that internal tabs are passed through as literal tabs, not expanded to spaces.)

chenzhiguang commented 1 year ago

I have added some more comment for the tabRemaining property of Line.

Sorry for this back and forth, it has been a few months since I created this Line type, I should have written a detailed comment.

chenzhiguang commented 1 year ago

Should we continue? I am blocked by this PR.

kevmoo commented 1 year ago

@srawlins should really chime in here

chenzhiguang commented 1 year ago

What do we say?

chenzhiguang commented 1 year ago

@srawlins Hi Sam, I need your decision in order to plan the next steps. cc @kevmoo

srawlins commented 1 year ago

I will try to get to this today.

chenzhiguang commented 1 year ago

@srawlins Hi Sam, please don’t forget this one.

chenzhiguang commented 1 year ago

No worries! Thanks!