Remove redundant platform-specific dark theme processors#3874
Conversation
|
This pull request changes some projects for the first time in this development cycle. An additional commit containing all the necessary changes was pushed to the top of this PR's branch. To obtain these changes (for example if you want to push more changes) either fetch from your fork or apply the git patch. Git patchFurther information are available in Common Build Issues - Missing version increments. |
9799983 to
f89bc47
Compare
|
It would seem better if you experimented locally in your workspace until it’s working well rather than in a public draft PR that generates yet more email about interim states of progress that are, in my opinion, best managed by you privately until it’s ready for prime time. I don’t think verification builds are providing value in this case, do they? Perhaps such builds only contribute to global warming and to email generation. I’m tempted to ask about all the things I see being deleted, but it’s a draft so maybe pointless it’s a pointless question. |
f7d8536 to
9799983
Compare
By using the dark API from SWT, we can delete the processors in platform.ui. The processors were doing it platform specific. Once this is in we can delete the fragment projects. |
|
So much mail. Please can this be reduced? Are the repeated builds and cascading notifications necessary? It just seems to be getting worse by the day. So many activities in parallel in public, driving endless builds and around the clock notifications. Please, please reconsider your work patterns. Complete the design and testing before feeling the urge to create yet another draft on yet another topic in parallel to several others. |
Sure, I think you said the same in #3881 (comment). |
3932688 to
f8ac0a8
Compare
|
Testing this now in a custom aggregator build. |
f8ac0a8 to
be7d944
Compare
Display.setDarkThemePreferred(boolean) is a cross-platform SWT API that signals the OS dark mode preference, and the ThemeEngine already calls it whenever the CSS theme changes. The platform-specific dark theme processors, which toggle the same OS preference through internal, reflection-based hooks, are therefore redundant. Remove the dark theme processors: - org.eclipse.e4.ui.swt.gtk (DarkThemeProcessor using OS.setDarkThemePreferred) - org.eclipse.e4.ui.swt.win32 (DarkThemeProcessor using OS.setTheme) - org.eclipse.e4.ui.workbench.renderers.swt.cocoa (CocoaDarkThemeProcessor using OS.setTheme) The gtk and win32 fragments only existed to host these processors and are removed together with their entries in the org.eclipse.e4.rcp feature. The cocoa fragment is kept because it still provides other processors. The early display.setDarkThemePreferred(true) call and the manual styling of the workspace selection dialog in IDEApplication are left untouched, since the CSS engine is not yet running at that point in the startup sequence. Fixes eclipse-platform#3857
be7d944 to
d0994ee
Compare
|
Tested on Linux and Windows (I have no access to Mac) in an custom aggregator build and styling still works and OS components, like scrollbars in Windows are still styled. @HannesWell any concerns from the releng side? AFAICS these fragments are not part of a pom.xml and I removed the reference to them in the feature.xml I plan to merge early next week if I do not hear concerns. |
HannesWell
left a comment
There was a problem hiding this comment.
It's nice to have the build and platform a bit simpler with this. Thanks for that.
And considering the theme earlier is also good. 👍🏽
any concerns from the releng side? AFAICS these fragments are not part of a pom.xml and I removed the reference to them in the feature.xml
From RelEng POV this change should be fine. I didn't find anymore relevant references.
I just see that in SWT there seem to be code, that could be obsolete now:
https://github.com/eclipse-platform/eclipse.platform.swt/blob/cb31ff97da4f842ad7c65af7dff65460178880c6/bundles/org.eclipse.swt/Eclipse%20SWT%20PI/gtk/org/eclipse/swt/internal/gtk/OS.java#L2030-L2041

The cross-platform SWT API
Display.setDarkThemePreferred(boolean)already drives native dark mode (title bars, scrollbars, native dialogs), and theThemeEnginecalls it whenever the CSS theme changes.That makes the platform-specific dark theme processors for GTK, Win32 and Cocoa redundant, so they are removed along with the fragile internal/reflection-based hooks they relied on.
The GTK and Win32 fragments only existed to host their processor and are deleted together with their entries in the
org.eclipse.e4.rcpfeature; the Cocoa fragment is kept because it still provides other processors.The early
Display.setDarkThemePreferred(true)call and the manual styling of the workspace selection dialog inIDEApplicationare intentionally left in place, because the CSS engine is not yet running at that point in startup.Fixes #3857