Skip to content

Ignore inline notes during Markdown reference parsing#1713

Merged
tompng merged 2 commits into
ruby:masterfrom
skatkov:markdown-note-state-reset
May 24, 2026
Merged

Ignore inline notes during Markdown reference parsing#1713
tompng merged 2 commits into
ruby:masterfrom
skatkov:markdown-note-state-reset

Conversation

@skatkov
Copy link
Copy Markdown
Contributor

@skatkov skatkov commented May 14, 2026

Inline notes inside reference labels can be parsed during the reference-gathering pass, before footnote ordering is initialized.

RDoc::Markdown.parse("[foo ^[note]]: /url\n")

This will crash a Markdown parser with a NoMethodError.

This change treats inline-note creation as a no-op until note ordering exists.

Copilot AI review requested due to automatic review settings May 14, 2026 15:05
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Resets @footnotes and @note_order to nil at the start of each parse call, and makes the InlineNote rule raise a ParseError when invoked before notes are initialized (e.g., during the references-gathering pass on a reused parser instance).

Changes:

  • Reset @footnotes and @note_order to nil at the start of parse to prevent leakage across reused parser instances.
  • Update the InlineNote PEG rule action to raise ParseError, 'invalid inline note' when @note_order is nil.
  • Add a regression test exercising an inline note inside a reference label after a prior parse.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
lib/rdoc/markdown.kpeg Source grammar: clear note state on parse and guard InlineNote action.
lib/rdoc/markdown.rb Generated parser mirroring the kpeg changes (state reset + guard + Rules entry).
test/rdoc/rdoc_markdown_test.rb New test validating the ParseError is raised on inline notes within reference labels after reuse.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread lib/rdoc/markdown.kpeg Outdated
Comment thread lib/rdoc/markdown.kpeg Outdated
@skatkov skatkov changed the title Reset Markdown note state between parses Ignore inline notes during Markdown reference parsing May 14, 2026
@skatkov skatkov requested a review from tompng May 14, 2026 20:29
@skatkov skatkov temporarily deployed to fork-preview-protection May 23, 2026 06:26 — with GitHub Actions Inactive
@matzbot
Copy link
Copy Markdown
Collaborator

matzbot commented May 23, 2026

🚀 Preview deployment available at: https://2ad84890.rdoc-6cd.pages.dev (commit: 6549e8b)

@tompng
Copy link
Copy Markdown
Member

tompng commented May 24, 2026

Looks good. Could you resolve the conflicts?

skatkov added 2 commits May 24, 2026 19:05
Inline notes inside reference labels can run during the reference-gathering pass. Reset note state for each parse and reject inline notes before note ordering starts so reused parser instances do not accept stale footnote data.
Inline notes can be parsed while Markdown references are being collected, before footnote ordering has been initialized. Treat that early pass as a no-op so malformed reference labels do not crash the parser.
@skatkov skatkov force-pushed the markdown-note-state-reset branch from c30a018 to 6549e8b Compare May 24, 2026 17:07
Copilot AI review requested due to automatic review settings May 24, 2026 17:07
@skatkov skatkov deployed to fork-preview-protection May 24, 2026 17:07 — with GitHub Actions Active
@skatkov
Copy link
Copy Markdown
Contributor Author

skatkov commented May 24, 2026

Looks good. Could you resolve the conflicts?

All conflicts resolved.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

Comment thread lib/rdoc/markdown.kpeg
Comment on lines +1239 to +1244
{ if @note_order
ref = [:inline, @note_order.length]
@footnotes[ref] = paragraph a

note_for ref
note_for ref
end
Copy link
Copy Markdown
Contributor Author

@skatkov skatkov May 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's the intention. It should be a no-op

def test_parse_note_inline_in_reference_label
@parser.notes = true

assert_kind_of RDoc::Markup::Document, parse("[foo ^[note]]: /url\n")
Copy link
Copy Markdown
Member

@tompng tompng left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you 👍

@tompng tompng merged commit 0a388a7 into ruby:master May 24, 2026
30 checks passed
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.

4 participants