Skip to content

Remove redundant @text_files_hash - #1814

Merged
tompng merged 2 commits into
ruby:masterfrom
tompng:rm_text_files_hash
Sep 7, 2026
Merged

Remove redundant @text_files_hash#1814
tompng merged 2 commits into
ruby:masterfrom
tompng:rm_text_files_hash

Conversation

@tompng

@tompng tompng commented Sep 6, 2026

Copy link
Copy Markdown
Member

@files_hash + page.text? is enough.

Copilot AI balanced review requested due to automatic review settings September 6, 2026 06:24
@github-actions

github-actions Bot commented Sep 6, 2026

Copy link
Copy Markdown

Documentation preview

View the preview

Commit: 55f31f8

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Approval recommended

The removed cache has no remaining references, and the new find_text_page logic preserves expected behavior while simplifying state management.

Pull request overview

This PR simplifies RDoc::Store by removing the dedicated @text_files_hash cache and instead using @files_hash plus TopLevel#text? to identify text pages on demand.

Changes:

  • Removed @text_files_hash tracking and the associated cache_text_file method from RDoc::Store.
  • Simplified Store#find_text_page to fetch from @files_hash and return the page only when it is a text page (text?).
  • Removed TopLevel#parser=’s side-effect (updating the removed cache) and switched parser to attr_accessor.
File summaries
File Description
lib/rdoc/store.rb Removes the redundant text-page cache and makes find_text_page rely on @files_hash + text?.
lib/rdoc/code_object/top_level.rb Removes cache-updating behavior from parser= now that text-page caching is gone.
Review details
  • Files reviewed: 2/2 changed files
  • Comments generated: 0
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread lib/rdoc/store.rb Outdated
Copilot AI review requested due to automatic review settings September 7, 2026 12:30
@tompng

tompng commented Sep 7, 2026

Copy link
Copy Markdown
Member Author

@copilot resolve the merge conflicts in this pull request

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Approval recommended

The simplified lookup preserves existing behavior without leaving stale references to the removed cache.

Review details
  • Files reviewed: 2/2 changed files
  • Comments generated: 0 new
  • Review effort level: Balanced

tompng and others added 2 commits September 7, 2026 21:41
Copilot AI review requested due to automatic review settings September 7, 2026 12:42
@tompng
tompng force-pushed the rm_text_files_hash branch from 7052e6c to 55f31f8 Compare September 7, 2026 12:42

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Approval recommended

The primary file index preserves existing lookup behavior without redundant state.

Review details
  • Files reviewed: 2/2 changed files
  • Comments generated: 0 new
  • Review effort level: Balanced

@tompng
tompng merged commit d26b5cf into ruby:master Sep 7, 2026
29 checks passed
@tompng
tompng deleted the rm_text_files_hash branch September 7, 2026 12:54
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.

3 participants