-
Notifications
You must be signed in to change notification settings - Fork 56
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
IllegalStateException in AbstractThemeManager.getDefaultTheme() #721
Comments
That is nowhere specified. Most of the code in the tm4e.ui package expects to be running on the UI thread. Anyways, I added a workaround for this case. Can you please give the latest snapshot build a shot? https://download.eclipse.org/tm4e/snapshots/ |
A first check seems very promising. @sebthom Thank you for the fast fix! Has the default theme changed as well? |
The default theme depends on the Eclipse UI theme and should only return the dark theme if Eclipse itself is dark. This is the integration test: tm4e/org.eclipse.tm4e.ui.tests/src/main/java/org/eclipse/tm4e/ui/tests/themes/ThemeManagerTest.java Lines 33 to 34 in 33dbe01
What has changed is, that we now don't try to guess the Eclipse theme based on theme names but by measuring brightness of the active shell's background color. tm4e/org.eclipse.tm4e.ui/src/main/java/org/eclipse/tm4e/ui/internal/utils/UI.java Lines 163 to 181 in 33dbe01
Maybe this does not work in some scenarios. It is working fine for me on Windows 10. |
I released TM4E 0.10.3 which contains the fix. Thanks for reporting the issue! |
Due to a failed unit test in LSP4E it turns out that since this TM4E commit a
IllegalStateException
will be thrown byAbstractThemeManager.getDefaultTheme()
, when this method gets called from within a non UI thread:tm4e/org.eclipse.tm4e.ui/src/main/java/org/eclipse/tm4e/ui/internal/themes/AbstractThemeManager.java
Line 57 in 039aed1
I am not sure if this should be fixed here or in the caller in LSP4E. Should the user be aware, that this method should be called from within a UI thread only?:
The text was updated successfully, but these errors were encountered: