Copilot commented on code in PR #2469:
URL: https://github.com/apache/groovy/pull/2469#discussion_r3097078348
##########
subprojects/groovy-console/src/main/groovy/groovy/console/ui/Console.groovy:
##########
@@ -283,8 +285,8 @@ class Console implements CaretListener, HyperlinkListener,
ComponentListener, Fo
// full stack trace should not be logged to the output window -
GROOVY-4663
Logger.getLogger(StackTraceUtils.STACK_LOG_NAME).useParentHandlers =
false
- //when starting via main set the look and feel to system
- UIManager.setLookAndFeel(UIManager.getSystemLookAndFeelClassName())
+ //set the look and feel based on theme preference
+ ThemeManager.applyTheme(ThemeManager.currentMode)
Review Comment:
This new startup path calls
`ThemeManager.applyTheme(ThemeManager.currentMode)`, but there is no
`groovy.console.ui.ThemeManager` implementation present in the repository.
As-is, groovyConsole won’t compile/run. Add the missing ThemeManager (including
`currentMode` and `applyTheme`) or restore the previous
`UIManager.setLookAndFeel(UIManager.getSystemLookAndFeelClassName())` behavior.
```suggestion
// set the look and feel to the system default
UIManager.setLookAndFeel(UIManager.getSystemLookAndFeelClassName())
```
##########
subprojects/groovy-console/src/main/groovy/groovy/console/ui/view/MacOSXMenuBar.groovy:
##########
@@ -195,6 +195,13 @@ menuBar {
checkBoxMenuItem(detachedOutputAction, selected:
controller.detachedOutput)
checkBoxMenuItem(autoClearOutputAction, selected:
controller.autoClearOutput)
checkBoxMenuItem(orientationVerticalAction, selected:
controller.orientationVertical)
+ separator()
+ menu(text: 'Theme') {
+ buttonGroup(id: 'themeGroup')
+ radioButtonMenuItem(lightThemeAction, buttonGroup: themeGroup,
selected: controller.currentTheme == 'LIGHT')
+ radioButtonMenuItem(darkThemeAction, buttonGroup: themeGroup,
selected: controller.currentTheme == 'DARK')
+ radioButtonMenuItem(systemThemeAction, buttonGroup: themeGroup,
selected: controller.currentTheme == 'SYSTEM')
Review Comment:
Same as the BasicMenuBar: these `selected:` expressions are only applied at
construction time, so theme changes triggered outside the menu (e.g., toolbar
cycle button) won’t update the checked radio item. Bind selection to
`controller.currentTheme` or update the menu items programmatically when
`switchTheme` runs.
```suggestion
radioButtonMenuItem(lightThemeAction, buttonGroup: themeGroup,
selected: bind(source: controller, sourceProperty:
'currentTheme', converter: { it == 'LIGHT' }))
radioButtonMenuItem(darkThemeAction, buttonGroup: themeGroup,
selected: bind(source: controller, sourceProperty:
'currentTheme', converter: { it == 'DARK' }))
radioButtonMenuItem(systemThemeAction, buttonGroup: themeGroup,
selected: bind(source: controller, sourceProperty:
'currentTheme', converter: { it == 'SYSTEM' }))
```
##########
subprojects/groovy-console/src/main/groovy/groovy/console/ui/view/BasicMenuBar.groovy:
##########
@@ -69,6 +69,13 @@ menuBar {
checkBoxMenuItem(detachedOutputAction, selected:
controller.detachedOutput)
checkBoxMenuItem(autoClearOutputAction, selected:
controller.autoClearOutput)
checkBoxMenuItem(orientationVerticalAction, selected:
controller.orientationVertical)
+ separator()
+ menu(text: 'Theme') {
+ buttonGroup(id: 'themeGroup')
+ radioButtonMenuItem(lightThemeAction, buttonGroup: themeGroup,
selected: controller.currentTheme == 'LIGHT')
+ radioButtonMenuItem(darkThemeAction, buttonGroup: themeGroup,
selected: controller.currentTheme == 'DARK')
+ radioButtonMenuItem(systemThemeAction, buttonGroup: themeGroup,
selected: controller.currentTheme == 'SYSTEM')
Review Comment:
The radio items’ `selected:` values are only evaluated when the menu is
built. When the theme is changed via the toolbar’s `cycleThemeAction`,
`controller.currentTheme` changes but these menu items won’t automatically
update selection state, leaving the View → Theme menu out of sync. Consider
binding `selected` to `controller.currentTheme` (with a property change /
binding) or explicitly updating the menu selection in
`switchTheme`/`reapplyStyles`.
```suggestion
radioButtonMenuItem(lightThemeAction, buttonGroup: themeGroup,
selected: bind(source: controller, sourceProperty:
'currentTheme', converter: { it == 'LIGHT' }))
radioButtonMenuItem(darkThemeAction, buttonGroup: themeGroup,
selected: bind(source: controller, sourceProperty:
'currentTheme', converter: { it == 'DARK' }))
radioButtonMenuItem(systemThemeAction, buttonGroup: themeGroup,
selected: bind(source: controller, sourceProperty:
'currentTheme', converter: { it == 'SYSTEM' }))
```
##########
subprojects/groovy-console/src/main/groovy/groovy/console/ui/view/Defaults.groovy:
##########
@@ -33,56 +30,4 @@ statusBarClass = BasicStatusBar
def prefs = Preferences.userNodeForPackage(Console)
def fontFamily = prefs.get("fontName", "Monospaced")
-styles = [
- // output window styles
- regular: [
- (StyleConstants.FontFamily): fontFamily
- ],
- prompt: [
- (StyleConstants.Foreground): new Color(0, 128, 0)
- ],
- command: [
- (StyleConstants.Foreground): Color.BLUE
- ],
- stacktrace: [
- (StyleConstants.Foreground): Color.RED.darker()
- ],
- hyperlink: [
- (StyleConstants.Foreground): Color.BLUE,
- (StyleConstants.Underline): true
- ],
- output: [:],
- result: [
- (StyleConstants.Foreground): Color.BLUE,
- (StyleConstants.Background): Color.YELLOW
- ],
-
- // syntax highlighting styles
- (StyleContext.DEFAULT_STYLE) : [
- (StyleConstants.FontFamily): fontFamily
- ],
- (GroovyFilter.COMMENT): [
- (StyleConstants.Foreground): Color.LIGHT_GRAY.darker().darker(),
- (StyleConstants.Italic) : true
- ],
- (GroovyFilter.QUOTES): [
- (StyleConstants.Foreground): Color.MAGENTA.darker().darker()
- ],
- (GroovyFilter.SINGLE_QUOTES): [
- (StyleConstants.Foreground): Color.GREEN.darker().darker()
- ],
- (GroovyFilter.SLASHY_QUOTES): [
- (StyleConstants.Foreground): Color.ORANGE.darker()
- ],
- (GroovyFilter.DIGIT): [
- (StyleConstants.Foreground): Color.RED.darker()
- ],
- (GroovyFilter.OPERATION): [
- (StyleConstants.Bold): true
- ],
- (GroovyFilter.IDENT): [:],
- (GroovyFilter.RESERVED_WORD): [
- (StyleConstants.Bold): true,
- (StyleConstants.Foreground): Color.BLUE.darker().darker()
- ]
-]
+styles = ThemeManager.getStyles(fontFamily)
Review Comment:
`ThemeManager` is referenced here, but there is no
`groovy.console.ui.ThemeManager` implementation in the codebase (searching the
repo finds no class/file defining it). This will fail compilation at `import
groovy.console.ui.ThemeManager` and at `ThemeManager.getStyles(fontFamily)`.
Add the missing ThemeManager (and ensure it’s included in the groovy-console
source set), or revert these references to the prior inlined style definitions.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
