Footnote support

[lindeer]

implementation referred to markdown-it-footnote and pandoc

[google-cla]

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

[lindeer]

hi @srawlins , I revised the code for check jobs, could you please have another review?

[chenzhiguang]

I created a footnote reference specification, it has some slightly different from some of the implementations. But at least there are clear rules: 🙂

https://github.com/chenzhiguang/dart_markdown/wiki/footnote-reference-specification

[chenzhiguang]

Test cases:
https://github.com/chenzhiguang/dart_markdown/blob/master/test/extensions/footnote_reference.unit

[lindeer]

thanks @chenzhiguang , I think footnote is an import markdown feature, especially in article writing though not in spec, not understand why official library do not support it, nearly every other language md library has it! But I find your implementation has huge difference from markdown-it and pandoc, and your based dart-lang md library has also great changes. I have a little concern for future maintenance.

[srawlins]

I think the primary spec we would follow (barring commonmark, since that does not support footnotes) would be GitHub (via https://github.blog/changelog/2021-09-30-footnotes-now-supported-in-markdown-fields/). I do really like pandoc, and think it is a popular project. I have not heard of markdown-it. So if we land an implementation it will have to be compatible with GitHub with a good array of tests.

The good array of tests is needed since GitHub does not have a spec for their footnote support (😢 ). That is still a bummer because we just have to... manually test with Babelmark or something? 😢

[lindeer]

Fine @srawlins , I would refine the code according to github's example. The main difference is paragraph making in footnote-definition part, and I will add more test cases.

[srawlins]

Thanks for the PR!

[srawlins]

@lindeer thanks for the new commit! Since it was force-pushed and overwrote the previous content, I can't see what was changed. Could you go through my comments and say something like "Done" on each if it has been addressed, and click "Resolve conversation" on those. I think I had a question or two as well which you could answer inline (I don't think I was specifically asking for code comments...) Thanks!

[lindeer]

sorry srawlins, I squashed commits too early, and github not addressed automatically. I think most important part of new commits was the introducing of 'reparse' in InlineParser. When failed to match link, we check if current content match footnote between '[' and ']', "[...][“ and "]", this result in that some leading content lost, so rollback 'pos' and 'start' to starting position and do some clearing things. Other parts reuse link syntax logic, trying to do minimum changes.

And I added many corner cases, some different from github (I commented unlike github), but that's because of our own design.

[lindeer]

hi @srawlins ,I found my way of 'reparse' implementation was not elegant, we could modify close() of LinkSyntax without any change of InlineParser. So could I drop this PR and create 2 separated PRs?

1. first one for modifying existing code without any regression.
2. after the first was merged, the second rebased onto the first, and was to implement footnote.

This way could make code modification more clear to review, could I?

[srawlins]

@lindeer That's not a bad idea. But you don't have to close this one first. If you create a PR for modifying LinkSyntax.close, I can see how it would benefit this PR.

[coveralls]

Pull Request Test Coverage Report for Build 3779626773

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

• For more information on this, see Tracking coverage changes with pull request builds.
• To avoid this issue with future PRs, see these Recommended CI Configurations.
• For a quick fix, rebase this PR at GitHub. Your next report should be accurate.

Details

• 115 of 117 (98.29%) changed or added relevant lines in 5 files are covered.
• 3 unchanged lines in 2 files lost coverage.
• Overall coverage increased (+0.2%) to 95.868%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
lib/src/document.dart	44	46	95.65%

Files with Coverage Reduction	New Missed Lines	%
lib/src/patterns.dart	1	93.75%
lib/src/document.dart	2	94.87%

Totals
Change from base Build 3707734073:	0.2%
Covered Lines:	1392
Relevant Lines:	1452

---

💛 - Coveralls

[srawlins]

Hi @lindeer I see you pushed last week. Is this ready for review again? I've lost context, apologies.

[lindeer]

@srawlins yes, it is. I just rebased onto new main branch, and added a new rebase fix, but github listed all commits.

[srawlins]

Great work! I have a few comments and then I will review further. Thanks again!

[lindeer]

hi @srawlins , seems every time I pushed a new commit, the review came into a pause, hope this could be merged earlier 😢

[srawlins]

Yes, apologies. We need to figure out a different system for open source contributions.

[srawlins]

This is looking so great! Thanks so much for working on this, I love it.

[lindeer]

@srawlins could you give another review? 🙏

[srawlins]

Almost there, thanks!

[srawlins]

Thanks so much for all of the doc links. That really helps ❤️ ❤️ ❤️

I love it, thanks!

[kevmoo]

We need to update the changelog and the version here, friends!

CC @lindeer @srawlins

[srawlins]

Will do :)