You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@netbeans.apache.org by "mbien (via GitHub)" <gi...@apache.org> on 2025/03/30 04:50:37 UTC
[PR] Remove hardcoded background default for ant property values. [netbeans]
mbien opened a new pull request, #8371:
URL: https://github.com/apache/netbeans/pull/8371
- this caused problems when the theme is copied: dark themes received bright backgrounds for ant property value expressions
- no default means "inherited" which seems to work better
symptom:
after fix:
--
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: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] Remove hardcoded background default for ant property values. [netbeans]
Posted by "eirikbakke (via GitHub)" <gi...@apache.org>.
eirikbakke commented on code in PR #8371:
URL: https://github.com/apache/netbeans/pull/8371#discussion_r2029936162
##########
java/ant.grammar/src/org/netbeans/modules/ant/grammar/AntHighlightsLayerFactory.java:
##########
@@ -38,18 +34,16 @@
public class AntHighlightsLayerFactory implements HighlightsLayerFactory {
public @Override HighlightsLayer[] createLayers(final Context context) {
- AttributeSet _attrs = MimeLookup.getLookup("text/x-jsp").lookup(FontColorSettings.class).getTokenFontColors("expression-language"); // NOI18N
- final AttributeSet attrs;
- if (_attrs == null) {
- // Fallback from web.core.syntax/src/org/netbeans/modules/web/core/syntax/resources/fontsColors.xml:
- SimpleAttributeSet _sattrs = new SimpleAttributeSet();
- _sattrs.addAttribute(StyleConstants.Background, new Color(0xe3f2e1));
- attrs = _sattrs;
- } else {
- attrs = _attrs;
- }
- return new HighlightsLayer[] {HighlightsLayer.create("ant", ZOrder.SYNTAX_RACK.forPosition(10), true,
- new AntHighlightsContainer((AbstractDocument) context.getDocument(), attrs))}; // NOI18N
+ HighlightsLayer highlighter = HighlightsLayer.create(
+ "ant", // NOI18N
+ ZOrder.SYNTAX_RACK.forPosition(10),
+ true,
+ new AntHighlightsContainer(
+ (AbstractDocument) context.getDocument(),
+ MimeLookup.getLookup("text/x-jsp").lookup(FontColorSettings.class).getTokenFontColors("expression-language") // NOI18N
Review Comment:
OK, so we're removing the fallback here, but the null value "means "inherited" which seems to work better". Makes sense!
--
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: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] Remove hardcoded background default for ant property values. [netbeans]
Posted by "mbien (via GitHub)" <gi...@apache.org>.
mbien merged PR #8371:
URL: https://github.com/apache/netbeans/pull/8371
--
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: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] Remove hardcoded background default for ant property values. [netbeans]
Posted by "mbien (via GitHub)" <gi...@apache.org>.
mbien commented on code in PR #8371:
URL: https://github.com/apache/netbeans/pull/8371#discussion_r2020244504
##########
java/ant.grammar/src/org/netbeans/modules/ant/grammar/AntHighlightsLayerFactory.java:
##########
@@ -38,18 +34,16 @@
public class AntHighlightsLayerFactory implements HighlightsLayerFactory {
public @Override HighlightsLayer[] createLayers(final Context context) {
- AttributeSet _attrs = MimeLookup.getLookup("text/x-jsp").lookup(FontColorSettings.class).getTokenFontColors("expression-language"); // NOI18N
- final AttributeSet attrs;
- if (_attrs == null) {
- // Fallback from web.core.syntax/src/org/netbeans/modules/web/core/syntax/resources/fontsColors.xml:
- SimpleAttributeSet _sattrs = new SimpleAttributeSet();
- _sattrs.addAttribute(StyleConstants.Background, new Color(0xe3f2e1));
- attrs = _sattrs;
- } else {
- attrs = _attrs;
- }
- return new HighlightsLayer[] {HighlightsLayer.create("ant", ZOrder.SYNTAX_RACK.forPosition(10), true,
- new AntHighlightsContainer((AbstractDocument) context.getDocument(), attrs))}; // NOI18N
+ HighlightsLayer highlighter = HighlightsLayer.create(
+ "ant", // NOI18N
+ ZOrder.SYNTAX_RACK.forPosition(10),
+ true,
+ new AntHighlightsContainer(
+ (AbstractDocument) context.getDocument(),
+ MimeLookup.getLookup("text/x-jsp").lookup(FontColorSettings.class).getTokenFontColors("expression-language") // NOI18N
Review Comment:
the fact that this uses the JSP EL setting for ant means that it isn't there if the module isn't loaded. My NB often disables some enterprise modules on startup due to ergonomics, in that case it loses its color settings until the module is activated again.
--
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: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] Remove hardcoded background default for ant property values. [netbeans]
Posted by "mbien (via GitHub)" <gi...@apache.org>.
mbien commented on code in PR #8371:
URL: https://github.com/apache/netbeans/pull/8371#discussion_r2029970329
##########
java/ant.grammar/src/org/netbeans/modules/ant/grammar/AntHighlightsLayerFactory.java:
##########
@@ -38,18 +34,16 @@
public class AntHighlightsLayerFactory implements HighlightsLayerFactory {
public @Override HighlightsLayer[] createLayers(final Context context) {
- AttributeSet _attrs = MimeLookup.getLookup("text/x-jsp").lookup(FontColorSettings.class).getTokenFontColors("expression-language"); // NOI18N
- final AttributeSet attrs;
- if (_attrs == null) {
- // Fallback from web.core.syntax/src/org/netbeans/modules/web/core/syntax/resources/fontsColors.xml:
- SimpleAttributeSet _sattrs = new SimpleAttributeSet();
- _sattrs.addAttribute(StyleConstants.Background, new Color(0xe3f2e1));
- attrs = _sattrs;
- } else {
- attrs = _attrs;
- }
- return new HighlightsLayer[] {HighlightsLayer.create("ant", ZOrder.SYNTAX_RACK.forPosition(10), true,
- new AntHighlightsContainer((AbstractDocument) context.getDocument(), attrs))}; // NOI18N
+ HighlightsLayer highlighter = HighlightsLayer.create(
+ "ant", // NOI18N
+ ZOrder.SYNTAX_RACK.forPosition(10),
+ true,
+ new AntHighlightsContainer(
+ (AbstractDocument) context.getDocument(),
+ MimeLookup.getLookup("text/x-jsp").lookup(FontColorSettings.class).getTokenFontColors("expression-language") // NOI18N
Review Comment:
right. It will use the base colors if the theme does not provide a color for it. FlatLaf light has a highlighting layer defined for those property values, the dark variant has not, so it will look like on the screenshot. Which is better than using the hardcoded value which was meant for light themes. This is especially a problem while trying to create your own theme.
thanks for reviewing!
--
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: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists
Re: [PR] Remove hardcoded background default for ant property values. [netbeans]
Posted by "mbien (via GitHub)" <gi...@apache.org>.
mbien commented on code in PR #8371:
URL: https://github.com/apache/netbeans/pull/8371#discussion_r2029970329
##########
java/ant.grammar/src/org/netbeans/modules/ant/grammar/AntHighlightsLayerFactory.java:
##########
@@ -38,18 +34,16 @@
public class AntHighlightsLayerFactory implements HighlightsLayerFactory {
public @Override HighlightsLayer[] createLayers(final Context context) {
- AttributeSet _attrs = MimeLookup.getLookup("text/x-jsp").lookup(FontColorSettings.class).getTokenFontColors("expression-language"); // NOI18N
- final AttributeSet attrs;
- if (_attrs == null) {
- // Fallback from web.core.syntax/src/org/netbeans/modules/web/core/syntax/resources/fontsColors.xml:
- SimpleAttributeSet _sattrs = new SimpleAttributeSet();
- _sattrs.addAttribute(StyleConstants.Background, new Color(0xe3f2e1));
- attrs = _sattrs;
- } else {
- attrs = _attrs;
- }
- return new HighlightsLayer[] {HighlightsLayer.create("ant", ZOrder.SYNTAX_RACK.forPosition(10), true,
- new AntHighlightsContainer((AbstractDocument) context.getDocument(), attrs))}; // NOI18N
+ HighlightsLayer highlighter = HighlightsLayer.create(
+ "ant", // NOI18N
+ ZOrder.SYNTAX_RACK.forPosition(10),
+ true,
+ new AntHighlightsContainer(
+ (AbstractDocument) context.getDocument(),
+ MimeLookup.getLookup("text/x-jsp").lookup(FontColorSettings.class).getTokenFontColors("expression-language") // NOI18N
Review Comment:
right. It will use the base colors if the theme does not provide a color for it. FlatLaf light has a highlighting layer defined for those property values, the dark variant has not, so it will look like on the screenshot. Which is better than using the hardcoded value which was meant for light themes. This was especially a problem while trying to create your own theme.
thanks for reviewing!
--
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: notifications-unsubscribe@netbeans.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org
For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists