-
Notifications
You must be signed in to change notification settings - Fork 19
Add support for footnotes #25
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
Conversation
lpil
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work! Thank you!
I've left a question inline. I think the code would benefit from some documentation comments as it has got more complex- there's a good deal more abstraction being applied after this patch than the previous design had. This can be OK, but it makes the code harder to understand and modify in future, so we need to make it as clear as possible.
Thank you.
P.S. You sentence mining project looks really cool! Maybe one day I'll figure you how to use it for learning Irish Gaeilge :)
|
Thanks for the review! I was able to factor out some of the abstraction because it turns out you can nest definitions (which I thought I had already checked but I guess not 🤷). Additionally, I noticed a niche case that wouldn't be handled correctly, so I fixed that and added a test for it. Also, best of luck with your Irish Gaeilge learning! |
lpil
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you!! Fab work!
This PR adds support for footnotes.
Due to the way footnotes work in Djot, I had to restructure parts of the existing code to add support for parsing footnote definition blocks and incrementing the footnote number for each reference in the generated html.