Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Labeled footnotes #90

Closed
wants to merge 3 commits into from
Closed

Labeled footnotes #90

wants to merge 3 commits into from

Conversation

sjml
Copy link

@sjml sjml commented Jan 13, 2020

This is very preliminary, so the code could use some cleanup before it gets merged, but I wanted to check in and see if there was interest before I polished it up. It might be something you want hidden behind an option if you want it at all.

Basically I missed the old Blackfriday behavior of footnotes having their labels in the links -- it's especially useful if you're trying to concatenate the output from multiple pages and want the return links to be unique.

When a patched goldmark is used with the Footnote extension, this Markdown:

Here is a document with a footnote.[^mynote]

Another footnote.[^othernote]

Numbered footnotes should still work.[^3]

I have a second link to the first note.[^mynote]

[^mynote]: Here is the footnote.
[^othernote]: Another one.
[^3]: A third footnote.

Gets rendered as:

<p>Here is a document with a footnote.<sup id="fnref:mynote"><a href="#fn:mynote" class="footnote-ref" role="doc-noteref">1</a></sup></p>
<p>Another footnote.<sup id="fnref:othernote"><a href="#fn:othernote" class="footnote-ref" role="doc-noteref">2</a></sup></p>
<p>Numbered footnotes should still work.<sup id="fnref:3"><a href="#fn:3" class="footnote-ref" role="doc-noteref">3</a></sup></p>
<p>I have a second link to the first note.<sup id="fnref:mynote"><a href="#fn:mynote" class="footnote-ref" role="doc-noteref">1</a></sup></p>
<section class="footnotes" role="doc-endnotes">
<hr>
<ol>
<li id="fn:mynote" role="doc-endnote">
<p>Here is the footnote. <a href="#fnref:mynote" class="footnote-backref" role="doc-backlink">&#x21a9;&#xfe0e;</a></p>
</li>
<li id="fn:othernote" role="doc-endnote">
<p>Another one. <a href="#fnref:othernote" class="footnote-backref" role="doc-backlink">&#x21a9;&#xfe0e;</a></p>
</li>
<li id="fn:3" role="doc-endnote">
<p>A third footnote. <a href="#fnref:3" class="footnote-backref" role="doc-backlink">&#x21a9;&#xfe0e;</a></p>
</li>
</ol>
</section>

Just a proof of concept; I don't know how well it fits your preferred patterns and coding style. Very open to feedback on that level, but wanted to check in on whether it was something that interests you before proceeding too much further.

Thanks for your great work on this project!

@sjml
Copy link
Author

sjml commented Jan 13, 2020

Ah, ok so the failed tests are on out-of-order footnote indices. That's definitely a behavior that would need to be accommodated. What I've got here roughly matches the way Blackfriday handles it, but I'll concede to your preference if you'd like me to continue with this.

(On my machine this makes all tests pass, but obviously it's now expecting different output, and I haven't run the whole test matrix like the GitHub workflow does.)

diff --git a/extension/_test/footnote.txt b/extension/_test/footnote.txt
index 9b06721..f4ea195 100644
--- a/extension/_test/footnote.txt
+++ b/extension/_test/footnote.txt
@@ -32,18 +32,18 @@ This[^3] is[^1] text with footnotes[^2].
 [^2]: Footnote two
 [^3]: Footnote three
 //- - - - - - - - -//
-<p>This<sup id="fnref:1"><a href="#fn:1" class="footnote-ref" role="doc-noteref">1</a></sup> is<sup id="fnref:2"><a href="#fn:2" class="footnote-ref" role="doc-noteref">2</a></sup> text with footnotes<sup id="fnref:3"><a href="#fn:3" class="footnote-ref" role="doc-noteref">3</a></sup>.</p>
+<p>This<sup id="fnref:3"><a href="#fn:3" class="footnote-ref" role="doc-noteref">1</a></sup> is<sup id="fnref:1"><a href="#fn:1" class="footnote-ref" role="doc-noteref">2</a></sup> text with footnotes<sup id="fnref:2"><a href="#fn:2" class="footnote-ref" role="doc-noteref">3</a></sup>.</p>
 <section class="footnotes" role="doc-endnotes">
 <hr>
 <ol>
+<li id="fn:3" role="doc-endnote">
+<p>Footnote three <a href="#fnref:3" class="footnote-backref" role="doc-backlink">&#x21a9;&#xfe0e;</a></p>
+</li>
 <li id="fn:1" role="doc-endnote">
-<p>Footnote three <a href="#fnref:1" class="footnote-backref" role="doc-backlink">&#x21a9;&#xfe0e;</a></p>
+<p>Footnote one <a href="#fnref:1" class="footnote-backref" role="doc-backlink">&#x21a9;&#xfe0e;</a></p>
 </li>
 <li id="fn:2" role="doc-endnote">
-<p>Footnote one <a href="#fnref:2" class="footnote-backref" role="doc-backlink">&#x21a9;&#xfe0e;</a></p>
-</li>
-<li id="fn:3" role="doc-endnote">
-<p>Footnote two <a href="#fnref:3" class="footnote-backref" role="doc-backlink">&#x21a9;&#xfe0e;</a></p>
+<p>Footnote two <a href="#fnref:2" class="footnote-backref" role="doc-backlink">&#x21a9;&#xfe0e;</a></p>
 </li>
 </ol>
 </section>

@sjml
Copy link
Author

sjml commented Jan 17, 2020

I went ahead and pushed changes to the test file so checks would pass, but in the meanwhile PR #93 came in which looks to do the same thing. It looks like more-or-less the same approach, so whichever one of these you feel better about, @yuin. (At a quick glance, @tv42's approach looks like better code, but I'm not sure how it handles ordering or interleaving numbered and labeled footnotes.)

@tv42
Copy link

tv42 commented Jan 17, 2020

Yeah, I didn't really think much about interleaving, and didn't want to disturb the existing code too much. I feel like named footnotes might need to have Index==-1 and not consume sequence numbers, to make interleaving work right.

@stale
Copy link

stale bot commented Feb 16, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@sjml
Copy link
Author

sjml commented Feb 17, 2020

Closed in favor of #92 / #93, though it looks like that might not get merged either...

@stale stale bot removed the stale label Feb 17, 2020
@sjml sjml closed this Feb 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants