You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@logging.apache.org by GitBox <gi...@apache.org> on 2022/06/15 11:28:30 UTC

[GitHub] [logging-log4j2] PashaTurok opened a new pull request, #933: LOG4J2-3538 Added support for 24 colors in highlighting

PashaTurok opened a new pull request, #933:
URL: https://github.com/apache/logging-log4j2/pull/933

   My first pull request. Please, check.


-- 
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@logging.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [logging-log4j2] PashaTurok commented on a diff in pull request #933: LOG4J2-3538 Added support for 24 colors in highlighting

Posted by GitBox <gi...@apache.org>.
PashaTurok commented on code in PR #933:
URL: https://github.com/apache/logging-log4j2/pull/933#discussion_r898424036


##########
log4j-core/src/main/java/org/apache/logging/log4j/core/pattern/AnsiEscape.java:
##########
@@ -320,12 +321,43 @@ public static String createSequence(final String... names) {
         boolean first = true;
         for (final String name : names) {
             try {
-                final AnsiEscape escape = EnglishEnums.valueOf(AnsiEscape.class, name.trim());
                 if (!first) {
                     sb.append(AnsiEscape.SEPARATOR.getCode());
                 }
                 first = false;
-                sb.append(escape.getCode());
+                String hexColor = null;
+                final String trimmedName = name.trim();
+                if (trimmedName.startsWith("#")) {
+                    sb.append("38");
+                    sb.append(SEPARATOR.getCode());
+                    sb.append("2");
+                    sb.append(SEPARATOR.getCode());
+                    hexColor = trimmedName;
+                } else if (trimmedName.startsWith("FG_#")) {
+                    sb.append("38");
+                    sb.append(SEPARATOR.getCode());
+                    sb.append("2");
+                    sb.append(SEPARATOR.getCode());
+                    hexColor = trimmedName.substring(3);
+                } else if (trimmedName.startsWith("BG_#")) {
+                    sb.append("48");
+                    sb.append(SEPARATOR.getCode());
+                    sb.append("2");
+                    sb.append(SEPARATOR.getCode());
+                    hexColor = trimmedName.substring(3);
+                }
+                if (hexColor != null) {
+                    Color color = Color.decode(hexColor);
+                    sb.append(color.getRed());
+                    sb.append(SEPARATOR.getCode());
+                    sb.append(color.getGreen());
+                    sb.append(SEPARATOR.getCode());
+                    sb.append(color.getBlue());
+                    //no separator at the end

Review Comment:
   @ppkarwasz Please, check. 



-- 
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@logging.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [logging-log4j2] ppkarwasz merged pull request #933: LOG4J2-3538 Added support for 24 colors in highlighting

Posted by GitBox <gi...@apache.org>.
ppkarwasz merged PR #933:
URL: https://github.com/apache/logging-log4j2/pull/933


-- 
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@logging.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [logging-log4j2] ppkarwasz commented on a diff in pull request #933: LOG4J2-3538 Added support for 24 colors in highlighting

Posted by GitBox <gi...@apache.org>.
ppkarwasz commented on code in PR #933:
URL: https://github.com/apache/logging-log4j2/pull/933#discussion_r898730793


##########
log4j-core/src/main/java/org/apache/logging/log4j/core/pattern/AnsiEscape.java:
##########
@@ -320,12 +320,42 @@ public static String createSequence(final String... names) {
         boolean first = true;
         for (final String name : names) {
             try {
-                final AnsiEscape escape = EnglishEnums.valueOf(AnsiEscape.class, name.trim());
                 if (!first) {
                     sb.append(AnsiEscape.SEPARATOR.getCode());
                 }
                 first = false;
-                sb.append(escape.getCode());
+                String hexColor = null;
+                final String trimmedName = name.trim();

Review Comment:
   ```suggestion
                   final String trimmedName = name.trim().toUpperCase(Locale.ENGLISH);
   ```
   
   since the other keywords are also matched in a case insensitive way.



##########
log4j-core/src/main/java/org/apache/logging/log4j/core/pattern/AnsiEscape.java:
##########
@@ -320,12 +320,42 @@ public static String createSequence(final String... names) {
         boolean first = true;
         for (final String name : names) {
             try {
-                final AnsiEscape escape = EnglishEnums.valueOf(AnsiEscape.class, name.trim());
                 if (!first) {
                     sb.append(AnsiEscape.SEPARATOR.getCode());
                 }
                 first = false;
-                sb.append(escape.getCode());
+                String hexColor = null;
+                final String trimmedName = name.trim();
+                if (trimmedName.startsWith("#")) {
+                    sb.append("38");
+                    sb.append(SEPARATOR.getCode());
+                    sb.append("2");
+                    sb.append(SEPARATOR.getCode());
+                    hexColor = trimmedName;
+                } else if (trimmedName.startsWith("FG_#")) {
+                    sb.append("38");
+                    sb.append(SEPARATOR.getCode());
+                    sb.append("2");
+                    sb.append(SEPARATOR.getCode());
+                    hexColor = trimmedName.substring(3);
+                } else if (trimmedName.startsWith("BG_#")) {
+                    sb.append("48");
+                    sb.append(SEPARATOR.getCode());
+                    sb.append("2");
+                    sb.append(SEPARATOR.getCode());
+                    hexColor = trimmedName.substring(3);
+                }
+                if (hexColor != null) {
+                    sb.append(Integer.valueOf(hexColor.substring(1, 3), 16));//r
+                    sb.append(SEPARATOR.getCode());
+                    sb.append(Integer.valueOf(hexColor.substring(3, 5), 16));//g
+                    sb.append(SEPARATOR.getCode());
+                    sb.append(Integer.valueOf(hexColor.substring(5, 7), 16));//b
+                    //no separator at the end
+                } else {
+                    final AnsiEscape escape = EnglishEnums.valueOf(AnsiEscape.class, trimmedName);
+                    sb.append(escape.getCode());
+                }
             } catch (final Exception ex) {
                 // Ignore the error.
             }

Review Comment:
   ```suggestion
               } catch (final Exception ex) {
                   StatusLogger.getLogger().warn("The style attribute {} is incorrect.", name, ex);
               }
   ```
   
   The current convention is to log configuration errors.



-- 
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@logging.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [logging-log4j2] ppkarwasz commented on a diff in pull request #933: LOG4J2-3538 Added support for 24 colors in highlighting

Posted by GitBox <gi...@apache.org>.
ppkarwasz commented on code in PR #933:
URL: https://github.com/apache/logging-log4j2/pull/933#discussion_r897994239


##########
log4j-core/src/main/java/org/apache/logging/log4j/core/pattern/AnsiEscape.java:
##########
@@ -320,12 +321,43 @@ public static String createSequence(final String... names) {
         boolean first = true;
         for (final String name : names) {
             try {
-                final AnsiEscape escape = EnglishEnums.valueOf(AnsiEscape.class, name.trim());
                 if (!first) {
                     sb.append(AnsiEscape.SEPARATOR.getCode());
                 }
                 first = false;
-                sb.append(escape.getCode());
+                String hexColor = null;
+                final String trimmedName = name.trim();
+                if (trimmedName.startsWith("#")) {
+                    sb.append("38");
+                    sb.append(SEPARATOR.getCode());
+                    sb.append("2");
+                    sb.append(SEPARATOR.getCode());
+                    hexColor = trimmedName;
+                } else if (trimmedName.startsWith("FG_#")) {
+                    sb.append("38");
+                    sb.append(SEPARATOR.getCode());
+                    sb.append("2");
+                    sb.append(SEPARATOR.getCode());
+                    hexColor = trimmedName.substring(3);
+                } else if (trimmedName.startsWith("BG_#")) {
+                    sb.append("48");
+                    sb.append(SEPARATOR.getCode());
+                    sb.append("2");
+                    sb.append(SEPARATOR.getCode());
+                    hexColor = trimmedName.substring(3);
+                }
+                if (hexColor != null) {
+                    Color color = Color.decode(hexColor);
+                    sb.append(color.getRed());
+                    sb.append(SEPARATOR.getCode());
+                    sb.append(color.getGreen());
+                    sb.append(SEPARATOR.getCode());
+                    sb.append(color.getBlue());
+                    //no separator at the end

Review Comment:
   In version 3.x `log4j-core` will use classes from the `java.base` module only. To ease the migration of this PR to the `master` branch, can you rewrite it without `java.awt.Color`?



-- 
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@logging.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [logging-log4j2] PashaTurok commented on pull request #933: LOG4J2-3538 Added support for 24 colors in highlighting

Posted by GitBox <gi...@apache.org>.
PashaTurok commented on PR #933:
URL: https://github.com/apache/logging-log4j2/pull/933#issuecomment-1157297793

   @ppkarwasz Could you take a look why it failed? It seems the problem is that we added event logging for exception.


-- 
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@logging.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org