Footnotes implementation - #78
Conversation
trafico-bot
Bot
added
the
🚧 WIP
d9a392d to
ac17b63
Compare
trafico-bot
Bot
added
🔍 Ready for Review
|
If this is in a stable place, I'm fine to merge this now and sort out the issues in followup PRs. |
|
It implements some basic cases and doesn't seem to break anything at least :-) |
|
I am working now on finishing this. |
|
Weird, tests were passing before, fixing. |
|
Sigh, I asked the LLM to fix ameba warnings and it choose wrong on every one of them. Fixed now :-) |
|
Wonder if the Edit: Same with CLAUDE.md and the like |
Deleted and ignored. |
|
I can't spend any more time working on this PR. |
|
I can carry it from here if you want. |
That would be great, thanks. |
|
I may have some time to work on markpdf these days if the project is still alive. Is there any problem with this PR? |
|
Guess it's dead. If someone wants to pick up the footnotes implementation the code is there. |
|
Sorry about the tantrum. Costs me nothing to keep this PR open for whenever someone wants it. |
There was a problem hiding this comment.
Pull request overview
Implements GitHub Flavored Markdown (GFM) footnotes end-to-end in Markd: parsing inline footnote references and block footnote definitions, post-processing to normalize numbering / resolve nested references, and HTML rendering including backrefs.
Changes:
- Added a new block rule and node types to recognize and store footnote definitions and inline footnote references.
- Added a post-parse footnote processing pass to resolve references (including nesting), normalize numbering, and move definitions to the end.
- Implemented HTML output for footnote refs/definitions and enabled fixture-based coverage for the new behavior.
Reviewed changes
Copilot reviewed 10 out of 11 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/markd/rules/footnote_definition.cr | New block rule to match and collect footnote definition blocks. |
| src/markd/rule.cr | Adds footnote definition start regex and treats [ as “maybe special”. |
| src/markd/renderers/html_renderer.cr | Renders footnote refs/definitions and backrefs; supports valueless (boolean) attributes. |
| src/markd/renderer.cr | Adds abstract renderer hooks for footnote nodes (but currently has a duplicate abstract method). |
| src/markd/parsers/inline.cr | Parses [^label] as a new inline Footnote node in GFM mode. |
| src/markd/parsers/block.cr | Adds passes to parse definition contents as blocks and to resolve/number/move footnotes. |
| src/markd/node.cr | Introduces Footnote / FootnoteDefinition node types and container classification changes. |
| spec/fixtures/gfm-regression.txt | Enables existing regression fixtures that exercise footnotes (including nesting). |
| spec/fixtures/gfm-extensions.txt | Enables GFM extension fixtures for footnotes and escaping behavior. |
| .vscode/launch.json | Removes editor launch configuration from the repo. |
| .gitignore | Ignores spec binary and editor/agent files; broadens VS Code ignore to entire folder. |
Suppressed comments (1)
src/markd/parsers/block.cr:156
footnote_definitionsis deleted from insidefootnote_definitions.each, which can raise due to Hash mutation during iteration. Iterate overkeyssnapshot instead.
footnote_definitions.delete footnote_title
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
|
||
| ADMONITION_START = /^> \[!((?:NOTE|TIP|IMPORTANT|CAUTION|WARNING)+)](\s*.*)?$/ | ||
|
|
||
| FOOTNOTE_DEFINITION_START = /\[\^[^\]]+\]:(\s|$)/ |
| footnotes.each do |footnote_title, nodes| | ||
| if !footnote_definitions.has_key?(footnote_title) | ||
| nodes.each do |fn_node| | ||
| fn_node.type = Node::Type::Text | ||
| fn_node.text = "[^#{footnote_title}]" | ||
| end | ||
| footnotes.delete footnote_title | ||
| else |
| n, entering = event | ||
| if entering && n.type.text? | ||
| replaced = false | ||
| n.text = n.text.gsub(/\[\^([\w\-]+)\]/) do |m| |
| # Is it a footnote? | ||
| if is_footnote | ||
| # If the 1st char after the closing bracket is a ":" then it's NOT | ||
| # a footnote, it's a footnote definition. | ||
| if char_at?(@pos) == ':' |
|
I'll do a first round of review using Copilot first. @nobodywasishere hasn't been active in this project for a while—I wonder if she's been busy since joining the Crystal core team. |
|
I am still here. Over the past year though have been dealing with a lot of personal stuff, and wrestling with how programming is changing in the advent of AI. Hard to find the joy in doing things if there's a machine that'll do it much faster than you. |
Understand the feeling |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
|
Tried to accept the fix suggestions but after the 1st one now they are stale :-D I will get a minion to look at them |
- renderer.cr: remove duplicate thematic_break abstract method declaration - rule.cr: anchor FOOTNOTE_DEFINITION_START to line start so mid-line [^x]: does not incorrectly start a block-level definition - block.cr: iterate over a snapshot of hash keys instead of mutating footnotes/footnote_definitions during each-iteration (fixes potential Hash modified during iteration errors) - block.cr: broaden nested-footnote label regex from [\w-]+ to [^\]]+ so nested references with arbitrary label characters are detected - inline.cr: skip inline-link (...) parsing for footnote openers so [^a](url) renders as a footnote ref with (url) preserved as text
This is, AFAIK a full implementation of footnotes as per the specs we have on the repo.