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

Render footnotes #90

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Conversation

brandondyck
Copy link
Contributor

We can't render footnote definitions by calling cmark_render_html on each node separately, because that function relies upon counting footnotes definitions as it writes them. Instead, I mostly duplicated cmark-gfm's rendering logic; the only (intentional) differences are that I did not escape href attributes in footnote links (though this should probably be done eventually), and I didn't bother putting in all the whitespace around elements that cmark-gfm does.

This depends upon kristoff-it/supermd#9, and build.zig.zon will need to be updated once that is merged.

@kristoff-it
Copy link
Owner

Thank you for your PR(s). I've merged all the prerequisites to land footnote support in Zine, but for this last integration step I want to move in a different direction compared to what this PR does.

More specifically, direct integration forces the renderer to make too many decisions about how the HTML should look like. We should strive to minimize the amount of hardcoded IDs, classes and data attributes we generate.

I've put my design idea and suggestions in #92.

Feel free to update this PR towards that design or to start a new one as you see fit. If you do open a new PR, please close this one first.

If you decide to continue working on this, I'm happy to answer questions in both the aforementioned Issue and Discord as well.

@brandondyck
Copy link
Contributor Author

I haven't tested docgen yet, but rendering works.

@brandondyck brandondyck marked this pull request as draft November 13, 2024 16:44
@brandondyck brandondyck marked this pull request as ready for review November 14, 2024 19:56
@brandondyck
Copy link
Contributor Author

brandondyck commented Nov 14, 2024

Docgen is working correctly now. I compared the output with the original, and the only differences are the additions of Page.footnotes? and the Footnote type.
...except that I forgot to actually run it zine on my test site, and once I did I found that footnotes?'s type won't work:

---------- SCRIPT RESULT TYPE MISMATCH ----------
A script evaluated to an unexpected type.

This attribute expects to evaluate to one
of the following types:
   - bool

[script_eval_not_bool]
(post.shtml) C:\Users\Brandon Dyck\src\ratbeard.net\layouts\post.shtml:10:28:
    <section id="footnotes" :if="$page.footnotes?()">
    ```

@brandondyck
Copy link
Contributor Author

Fixed for real. This time I checked the docgen output and tested the footnotes? example code on pages with and without footnotes.

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