Skip to content

Merge AbstractCSSEngine into CSSEngineImpl#4040

Open
vogella wants to merge 1 commit into
eclipse-platform:masterfrom
vogella:css-merge-abstract-css-engine
Open

Merge AbstractCSSEngine into CSSEngineImpl#4040
vogella wants to merge 1 commit into
eclipse-platform:masterfrom
vogella:css-merge-abstract-css-engine

Conversation

@vogella
Copy link
Copy Markdown
Contributor

@vogella vogella commented May 31, 2026

The CSS core engine had two stacked abstract classes with no concrete core implementation: AbstractCSSEngine (~1,100 LOC) and a tiny CSSEngineImpl (~95 LOC) that only added parser-factory wiring, handler registration helpers, and the boolean value converter. This inlines the small subclass into the base and keeps the CSSEngineImpl name, since that is the type the SWT engine subclasses and the tests already reference. makeCSSParser becomes concrete (it was abstract only so the subclass could supply its single implementation); the class stays abstract because reapply() has no sensible core default.

The result is one fewer class in the engine hierarchy with no behaviour change. All packages are internal (x-friends only), so there is no API impact, and the existing CSS tests pass.

Contributes to #3980

The CSS core engine had two stacked abstract classes with no concrete
core implementation: AbstractCSSEngine (1,113 LOC) and a tiny
CSSEngineImpl (95 LOC) that only added parser-factory wiring,
property-handler registration helpers, and the boolean value
converter. Inline the small subclass into the base, keep the name
CSSEngineImpl since that is the type SWT engine subclasses and tests
already reference, and make makeCSSParser concrete (it was abstract
solely so CSSEngineImpl could supply its single implementation).
The class remains abstract because the CSSEngine.reapply() method has
no sensible default in the core bundle.

Update ParserTestUtil's parseCssWithoutImports cast and rename
AbstractCSSEngineTest to CSSEngineImplTest in the same pass to keep
test class names aligned with production class names. The test's
anonymous subclass no longer needs to override makeCSSParser since it
is now concrete on the base.

All bundles internal (x-friends only); no API surface changed.

Contributes to eclipse-platform#3980
@github-actions
Copy link
Copy Markdown
Contributor

Test Results

   864 files  ±0     864 suites  ±0   50m 44s ⏱️ - 1m 46s
 7 988 tests ±0   7 745 ✅ ±0  243 💤 ±0  0 ❌ ±0 
20 418 runs  ±0  19 763 ✅ ±0  655 💤 ±0  0 ❌ ±0 

Results for commit 9b24ad6. ± Comparison against base commit 1e6a7c9.

This pull request removes 1 and adds 1 tests. Note that renamed tests count towards both.
org.eclipse.e4.ui.css.core.impl.engine.AbstractCSSEngineTest ‑ testGetElement_null
org.eclipse.e4.ui.css.core.impl.engine.CSSEngineImplTest ‑ testGetElement_null

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.

1 participant