Skip to content

Simplify CrossReference resolve#1571

Open
tompng wants to merge 1 commit into
ruby:masterfrom
tompng:crossref_simplify
Open

Simplify CrossReference resolve#1571
tompng wants to merge 1 commit into
ruby:masterfrom
tompng:crossref_simplify

Conversation

@tompng
Copy link
Copy Markdown
Member

@tompng tompng commented Jan 19, 2026

Simply return nil if crossref resolve failed.
Crossref suppression (already done in another part) is not what CrossReference#resolve have to do.
So resolve(name, text) don't need text arg.
Cache mechanism of resolve will be simple: no string label cache, only cache ref or nil.

@tompng tompng temporarily deployed to fork-preview-protection January 19, 2026 18:38 — with GitHub Actions Inactive
@matzbot
Copy link
Copy Markdown
Collaborator

matzbot commented Jan 19, 2026

🚀 Preview deployment available at: https://61c3afb0.rdoc-6cd.pages.dev (commit: 3f5f873)

@tompng tompng force-pushed the crossref_simplify branch from a7f83c2 to f46ea96 Compare February 9, 2026 18:43
@tompng tompng temporarily deployed to fork-preview-protection February 9, 2026 18:43 — with GitHub Actions Inactive
@st0012
Copy link
Copy Markdown
Member

st0012 commented May 13, 2026

@tompng I think this needs a rebase

Copilot AI review requested due to automatic review settings May 13, 2026 16:36
@tompng tompng force-pushed the crossref_simplify branch from f46ea96 to 028d4bc Compare May 13, 2026 16:36
@tompng tompng temporarily deployed to fork-preview-protection May 13, 2026 16:36 — with GitHub Actions Inactive
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

This PR simplifies cross-reference resolution so unresolved or suppressed references return nil, with HTML rendering layers now responsible for fallback display behavior.

Changes:

  • Removes the text argument from RDoc::CrossReference#resolve.
  • Updates HTML cross-reference fallback handling and <tt> suppressed-reference behavior.
  • Adjusts tests for nil unresolved refs, escaped labels, and suppressed refs in <tt>.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.

File Description
lib/rdoc/cross_reference.rb Simplifies resolution return values and caching to refs or nil.
lib/rdoc/markup/to_html_crossref.rb Updates callers and HTML fallback logic for nil unresolved refs.
test/rdoc/rdoc_cross_reference_test.rb Updates resolver tests for the new nil-on-failure behavior.
test/rdoc/markup/to_html_crossref_test.rb Adds/updates HTML cross-reference tests for suppression and escaping.

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

Comment thread lib/rdoc/cross_reference.rb
Comment thread lib/rdoc/markup/to_html_crossref.rb
Comment thread lib/rdoc/markup/to_html_crossref.rb
Comment thread test/rdoc/markup/to_html_crossref_test.rb Outdated
Comment thread lib/rdoc/markup/to_html_crossref.rb Outdated
@tompng tompng force-pushed the crossref_simplify branch from 028d4bc to a6d6abc Compare May 14, 2026 13:57
@tompng tompng requested a deployment to fork-preview-protection May 14, 2026 13:57 — with GitHub Actions Waiting
Simply return nil if crossref resolve failed
Copilot AI review requested due to automatic review settings May 14, 2026 13:59
@tompng tompng force-pushed the crossref_simplify branch from a6d6abc to 3f5f873 Compare May 14, 2026 13:59
@tompng tompng deployed to fork-preview-protection May 14, 2026 14:00 — with GitHub Actions Active
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.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

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