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

Add MultiMarkdown footnotes extension #141

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

bdolman
Copy link

@bdolman bdolman commented Oct 31, 2012

The footnote syntax and output basically follows what's outlined here: http://michelf.ca/projects/php-markdown/extra/#footnotes

The parsing of the footnote definition is very similar to a list item, per the direction in the footnotes reference above.

New extension flag called MKDEXT_FOOTNOTES.

Three new rendering callbacks:

  • footnote_ref - The inline reference
  • footnote_def - The definition of an individual footnote at the end of the document
  • footnotes - The div, hr and list that wraps all of the footnote definitions at the end

The rest should be pretty self explanatory. I tried to follow the conventions in the current library. My buffer usage deserves some scrutiny to make sure I'm using the proper initial sizes.

I've done some basic testing, but nothing extensive. Would love to get some other people trying it and looking at it to see if I've missed something.

@bdolman bdolman mentioned this pull request Oct 31, 2012
@brief
Copy link
Contributor

brief commented Nov 4, 2012

Nice! I've run this again a dozen markdown files with footnotes. Haven't seen any errors yet.

@bdolman
Copy link
Author

bdolman commented Nov 8, 2012

Thanks for doing that brief! Glad to know that it's working for you.

@adamflorin
Copy link

I impatiently made a fork of Redcarpet to play with these features, as a client of mine needs them this week. (Lucky timing for me!)

So far so good. The HTML markup gave me a bit of trouble, though; jQuery doesn't seem to like namespaced IDs (id='fn:1'). (I'm building some custom JS-based footnote-display code, too.)

Will post here if I see any other issues, but it looks great, thanks @bdolman !

@adamflorin
Copy link

Hmm, one thing I just noticed is that it fails to recognize multiple footnotes if the line delimiter is \r\n\ and not just \n. @bdolman, try feeding it this and you'll see that it picks up the first footnote but not the second:

"Footnote in body text[^1].\r\n\r\nAnd another[^2].\r\n\r\n[^1]: Defined below.\r\n\r\n[^2]: But not this time.\r\n"

I'll just preprocess for now, but thought I'd mention it, as \r\n hasn't given me any trouble in Sundown yet!

@bdolman
Copy link
Author

bdolman commented Nov 9, 2012

@adamflorin Good catch! Give the latest a try.

@bdolman
Copy link
Author

bdolman commented Nov 9, 2012

@adamflorin I'm also not really a huge fan of the fn:1 style namespacing. I noticed that peg-markdown eschews fn:1 and fnref:1 in favor of fn1 and fnref1. I think I actually prefer that, especially if it prevents issues with JQuery et. al.

@microjo
Copy link

microjo commented Nov 19, 2012

+1 i'd like have this extension

@mildsunrise
Copy link

👍 yeah, we'd like the extension to be merged!
I have been waiting for this functionality in Sundown.

@vmg what do you think? Please consider adding it.

@bootstraponline
Copy link

+1

@adamflorin
Copy link

@bdolman, I confirmed that the \r\n issue is fixed by 511d6e9. Many thanks!

@vmg
Copy link
Owner

vmg commented Nov 29, 2012

Thanks for this PR, looks clean enough.

Unfortunately, Sundown is currently frozen as we're working with the Reddit, StackOverflow and Meteor developers to design a formal Markdown standard and parser that will supersede Sundown in all these websites (and in GitHub, of course). Our goal is to deprecate Sundown altogether before the end of the year. We're evaluating the possibility of adding a footnote extension to the new standard, but we're not quite there yet.

Sorry for the fuss. Stay tuned.

@mildsunrise
Copy link

estic preocupat. 😿
@vmg
Will it allow custom renderers and exts?
Will it be as fast and secure as this has proven?

@vmg
Copy link
Owner

vmg commented Nov 29, 2012

The only reason why we're deprecating Sundown is because we're 100% sure that we can come up with something faster, safer, and what's more important, more consistent than what we have now. I can assure you that the new parser will be an improvement in every sense -- and if it's not, you can always fork Sundown and continue developing it. Such is the magic of open source. :)

Just give me a couple weeks to get this thing going. I promise it'll be worth it. http://www.youtube.com/watch?v=FSJFTa2jvhk

@FSX
Copy link
Contributor

FSX commented Nov 29, 2012

Is there a place where we can follow the progress of this project, e.g. a mailinglist?

@mildsunrise
Copy link

@vmg Oh, lots of relaxed now.
You can't imagine how much.
Really. :)

@bdolman
Copy link
Author

bdolman commented Nov 29, 2012

Thanks for the update @vmg, I'm thrilled that you are working with those developers to nail down a markdown standard and an even better markdown renderer.

In the meantime I just want to let everyone know that this footnotes extension is being used in at least two popular apps (and probably more) without any known issues, so please feel confident in using it until we get the new shiny.

@devinus
Copy link

devinus commented Sep 25, 2013

@bdolman If you're interested, I have just released Hoedown 1.0.0, a revived fork of Sundown. It includes this pull request:

https://github.com/hoedown/hoedown

@bdolman
Copy link
Author

bdolman commented Sep 26, 2013

@devinus Awesome, thanks for picking it back up. I'll definitely be checking out hoedown. I like the name too :)

GerHobbelt pushed a commit to GerHobbelt/soldout that referenced this pull request Feb 11, 2021
dokutoku pushed a commit to dokutoku/sundown that referenced this pull request Mar 12, 2023
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.

9 participants