Skip to content

Clean up FontRegistry's legacy collection handling - #4268

Open
HeikoKlare wants to merge 1 commit into
eclipse-platform:masterfrom
HeikoKlare:fontregistry-multidisplay-step1e
Open

Clean up FontRegistry's legacy collection handling#4268
HeikoKlare wants to merge 1 commit into
eclipse-platform:masterfrom
HeikoKlare:fontregistry-multidisplay-step1e

Conversation

@HeikoKlare

Copy link
Copy Markdown
Contributor

Pure simplification of long-standing awkwardness in FontRegistry: collections handled as if generics did not exist (explicit Iterators, casts through Object), one variable in getFontRecord() reused for two unrelated values, and FontRecord reaching into the registry to retire its own fonts rather than just reporting which ones it allocated.

Also documents what cleanOnDisplayDisposal == false already promises.

No behaviour change, other than put() now invalidating the replaced record entirely before notifying listeners rather than partly after, so the registry is consistent by the time they run.

🤖 Generated with Claude Code

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This pull request simplifies FontRegistry collection handling and font-record lifecycle management, including invalidation before listener notifications.

Changes:

  • Replaces raw iterators and casts with typed collection APIs.
  • Refactors font allocation and disposal responsibilities.
  • Documents cleanOnDisplayDisposal == false behavior.
  • Ensures replaced records are invalidated before notifications.

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

FontRegistry still handles its collections the way it did before
generics: explicit Iterators, casts through Object, and one variable
reused for two unrelated values. It also lets FontRecord reach into the
registry to retire its own fonts, mixing up who owns that decision.

Simplify all of that, and write down what cleanOnDisplayDisposal ==
false already promises.

No behavior change, other than put() now invalidating the replaced
record entirely before notifying listeners rather than partly after, so
the registry is consistent by the time they run. That listeners already
see the new font when notified was untested, so a test now covers it.

Assisted-by: Claude Opus 5 <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown
Contributor

Test Results

   858 files  ±0     858 suites  ±0   46m 1s ⏱️ - 4m 32s
 8 173 tests +1   7 929 ✅ +1  243 💤 ±0  0 ❌ ±0  1 🔥 ±0 
20 421 runs  +3  19 764 ✅ +3  656 💤 ±0  0 ❌ ±0  1 🔥 ±0 

For more details on these errors, see this check.

Results for commit 719d847. ± Comparison against base commit 5b3e6dd.

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