Re: [edk2-devel] [PATCH 1/1] MdePkg: Use ANSI colors to indicate debug message severity
On 9/1/22 16:21, Andrew Fish via groups.io wrote: I think this feature flag is going to control the output of edk2 DEBUG. In an abstract sense DEBUG is not Terminal or ConOut. It could be mapped to the same device, but it does not have to be. So maybe PcdDebugAnsiSeqSupport or DebugLibAnsiSeqSupport? Thanks, that makes more sense. I'll send out a v3 patch with PcdDebugAnsiSeqSupport. -- Rebecca Cran -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#93070): https://edk2.groups.io/g/devel/message/93070 Mute This Topic: https://groups.io/mt/93337469/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH 1/1] MdePkg: Use ANSI colors to indicate debug message severity
> On Sep 1, 2022, at 4:52 PM, Rebecca Cran wrote: > > Thanks. I've just sent out a v2 that introduces a > > PcdTerminalSupportsAnsiSequences FeatureFlag. > > Let me know if you'd prefer a different name: I wondered about something like > PcdConOutAnsiSeqSupport for example. > Rebecca, I think this feature flag is going to control the output of edk2 DEBUG. In an abstract sense DEBUG is not Terminal or ConOut. It could be mapped to the same device, but it does not have to be. So maybe PcdDebugAnsiSeqSupport or DebugLibAnsiSeqSupport? Thanks, Andrew Fish > -- > Rebecca Cran > > On 8/29/22 17:29, Kinney, Michael D wrote: >> Hi Rebecca, >> >> I think this is a good idea to improve readability. >> >> However, I think there is an assumption today that debug >> message output is just ASCII text with no assumptions on >> a terminal type and no assumption on screen width or screen >> height and no support for cursor control. >> >> This change would only make sense if the device the >> serial messages are sent supports ANSI color codes. >> >> I think we should make this a build time configuration >> option to add the ANSI sequences. I suggest a FeatureFlag >> PCD with a default of disabled to preserve current behavior >> and platforms can opt into ANSI color coded debug messages >> in their DSC file. >> >> Thanks, >> >> Mike >> >>> -Original Message- >>> From: Rebecca Cran >>> Sent: Monday, August 29, 2022 3:12 PM >>> To: devel@edk2.groups.io; Kinney, Michael D ; >>> Gao, Liming ; Liu, Zhiguang >>> >>> Cc: Rebecca Cran >>> Subject: [PATCH 1/1] MdePkg: Use ANSI colors to indicate debug message >>> severity >>> >>> There currently isn't a way to differentiate the different >>> levels of DEBUG output: DEBUG_ERROR, DEBUG_WARN, DEBUG_INFO >>> etc. >>> >>> To improve this, wrap DEBUG_ERROR and DEBUG_WARN level >>> messages in ANSI color code escape sequences. DEBUG_ERROR >>> messages will be displayed in red text, and DEBUG_WARN >>> in orange (the escape code is for the yellow color palette >>> entry, but it gets displayed as orange). >>> >>> Signed-off-by: Rebecca Cran >>> --- >>> MdePkg/Include/Library/PrintLib.h| 39 ++ >>> MdePkg/Library/BaseDebugLibSerialPort/DebugLib.c | 2 + >>> MdePkg/Library/BasePrintLib/PrintLib.c | 128 >>> MdePkg/Library/UefiDebugLibConOut/DebugLib.c | 2 + >>> 4 files changed, 171 insertions(+) >>> >>> diff --git a/MdePkg/Include/Library/PrintLib.h >>> b/MdePkg/Include/Library/PrintLib.h >>> index 8d523cac528d..06d9761df897 100644 >>> --- a/MdePkg/Include/Library/PrintLib.h >>> +++ b/MdePkg/Include/Library/PrintLib.h >>> @@ -2,6 +2,7 @@ >>>Provides services to print a formatted string to a buffer. All >>> combinations of >>>Unicode and ASCII strings are supported. >>> >>> +Copyright (c) 2022, Qualcomm Innovation Center, Inc. All rights >>> reserved. >>> Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved. >>> SPDX-License-Identifier: BSD-2-Clause-Patent >>> >>> @@ -931,4 +932,42 @@ SPrintLengthAsciiFormat ( >>>IN VA_LIST Marker >>>); >>> >>> +/** >>> + Wraps a message with ANSI color escape codes. >>> + >>> + @param String The string to wrap. >>> + @param StringLen The size of the String buffer in characters. >>> + @param ErrorLevel The error level. >>> + >>> + @retval RETURN_SUCCESS The string was successfully updated. >>> + @retval RETURN_BUFFER_TOO_SMALL The buffer is too small. >>> + >>> +**/ >>> +RETURN_STATUS >>> +EFIAPI >>> +AsciiDebugGetColorString ( >>> + IN OUT CHAR8 *String, >>> + IN UINTN StringLen, >>> + IN UINTN ErrorLevel >>> + ); >>> + >>> +/** >>> + Wraps a message with ANSI color escape codes. >>> + >>> + @param String The string to wrap. >>> + @param StringLen The size of the String buffer in Unicode characters. >>> + @param ErrorLevel The error level. >>> + >>> + @retval RETURN_SUCCESS The string was successfully updated. >>> + @retval RETURN_BUFFER_TOO_SMALL The buffer is too small. >>> + >>> +**/ >>> +RETURN_STATUS >>> +EFIAPI >>> +UnicodeDebugGetColorString ( >>> + IN OUT CHAR16 *String, >>> + IN UINTN StringLen, >>> + IN UINTN ErrorLevel >>> + ); >>> + >>> #endif >>> diff --git a/MdePkg/Library/BaseDebugLibSerialPort/DebugLib.c >>> b/MdePkg/Library/BaseDebugLibSerialPort/DebugLib.c >>> index bd5686947712..d599d16f5b7a 100644 >>> --- a/MdePkg/Library/BaseDebugLibSerialPort/DebugLib.c >>> +++ b/MdePkg/Library/BaseDebugLibSerialPort/DebugLib.c >>> @@ -125,6 +125,8 @@ DebugPrintMarker ( >>> AsciiBSPrint (Buffer, sizeof (Buffer), Format, BaseListMarker); >>>} >>> >>> + AsciiDebugGetColorString (Buffer, MAX_DEBUG_MESSAGE_LENGTH, ErrorLevel); >>> + >>>// >>>// Send the print string to a Serial Port >>>// >>> diff --git a/MdePkg/Library/BasePrintLib/PrintLib.c >>> b/MdePkg/Library/BasePrintLib/PrintLib.c >>> index
Re: [edk2-devel] [PATCH 1/1] MdePkg: Use ANSI colors to indicate debug message severity
Thanks. I've just sent out a v2 that introduces a PcdTerminalSupportsAnsiSequences FeatureFlag. Let me know if you'd prefer a different name: I wondered about something like PcdConOutAnsiSeqSupport for example. -- Rebecca Cran On 8/29/22 17:29, Kinney, Michael D wrote: Hi Rebecca, I think this is a good idea to improve readability. However, I think there is an assumption today that debug message output is just ASCII text with no assumptions on a terminal type and no assumption on screen width or screen height and no support for cursor control. This change would only make sense if the device the serial messages are sent supports ANSI color codes. I think we should make this a build time configuration option to add the ANSI sequences. I suggest a FeatureFlag PCD with a default of disabled to preserve current behavior and platforms can opt into ANSI color coded debug messages in their DSC file. Thanks, Mike -Original Message- From: Rebecca Cran Sent: Monday, August 29, 2022 3:12 PM To: devel@edk2.groups.io; Kinney, Michael D ; Gao, Liming ; Liu, Zhiguang Cc: Rebecca Cran Subject: [PATCH 1/1] MdePkg: Use ANSI colors to indicate debug message severity There currently isn't a way to differentiate the different levels of DEBUG output: DEBUG_ERROR, DEBUG_WARN, DEBUG_INFO etc. To improve this, wrap DEBUG_ERROR and DEBUG_WARN level messages in ANSI color code escape sequences. DEBUG_ERROR messages will be displayed in red text, and DEBUG_WARN in orange (the escape code is for the yellow color palette entry, but it gets displayed as orange). Signed-off-by: Rebecca Cran --- MdePkg/Include/Library/PrintLib.h| 39 ++ MdePkg/Library/BaseDebugLibSerialPort/DebugLib.c | 2 + MdePkg/Library/BasePrintLib/PrintLib.c | 128 MdePkg/Library/UefiDebugLibConOut/DebugLib.c | 2 + 4 files changed, 171 insertions(+) diff --git a/MdePkg/Include/Library/PrintLib.h b/MdePkg/Include/Library/PrintLib.h index 8d523cac528d..06d9761df897 100644 --- a/MdePkg/Include/Library/PrintLib.h +++ b/MdePkg/Include/Library/PrintLib.h @@ -2,6 +2,7 @@ Provides services to print a formatted string to a buffer. All combinations of Unicode and ASCII strings are supported. +Copyright (c) 2022, Qualcomm Innovation Center, Inc. All rights reserved. Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved. SPDX-License-Identifier: BSD-2-Clause-Patent @@ -931,4 +932,42 @@ SPrintLengthAsciiFormat ( IN VA_LIST Marker ); +/** + Wraps a message with ANSI color escape codes. + + @param String The string to wrap. + @param StringLen The size of the String buffer in characters. + @param ErrorLevel The error level. + + @retval RETURN_SUCCESS The string was successfully updated. + @retval RETURN_BUFFER_TOO_SMALL The buffer is too small. + +**/ +RETURN_STATUS +EFIAPI +AsciiDebugGetColorString ( + IN OUT CHAR8 *String, + IN UINTN StringLen, + IN UINTN ErrorLevel + ); + +/** + Wraps a message with ANSI color escape codes. + + @param String The string to wrap. + @param StringLen The size of the String buffer in Unicode characters. + @param ErrorLevel The error level. + + @retval RETURN_SUCCESS The string was successfully updated. + @retval RETURN_BUFFER_TOO_SMALL The buffer is too small. + +**/ +RETURN_STATUS +EFIAPI +UnicodeDebugGetColorString ( + IN OUT CHAR16 *String, + IN UINTN StringLen, + IN UINTN ErrorLevel + ); + #endif diff --git a/MdePkg/Library/BaseDebugLibSerialPort/DebugLib.c b/MdePkg/Library/BaseDebugLibSerialPort/DebugLib.c index bd5686947712..d599d16f5b7a 100644 --- a/MdePkg/Library/BaseDebugLibSerialPort/DebugLib.c +++ b/MdePkg/Library/BaseDebugLibSerialPort/DebugLib.c @@ -125,6 +125,8 @@ DebugPrintMarker ( AsciiBSPrint (Buffer, sizeof (Buffer), Format, BaseListMarker); } + AsciiDebugGetColorString (Buffer, MAX_DEBUG_MESSAGE_LENGTH, ErrorLevel); + // // Send the print string to a Serial Port // diff --git a/MdePkg/Library/BasePrintLib/PrintLib.c b/MdePkg/Library/BasePrintLib/PrintLib.c index e6f4042bb90b..95135d45aff9 100644 --- a/MdePkg/Library/BasePrintLib/PrintLib.c +++ b/MdePkg/Library/BasePrintLib/PrintLib.c @@ -1,12 +1,15 @@ /** @file Base Print Library instance implementation. + Copyright (c) 2022, Qualcomm Innovation Center, Inc. All rights reserved. Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved. Portions copyright (c) 2008 - 2009, Apple Inc. All rights reserved. SPDX-License-Identifier: BSD-2-Clause-Patent **/ +#include + #include "PrintLibInternal.h" // @@ -19,6 +22,19 @@ VA_LIST gNullVaList; #define ASSERT_UNICODE_BUFFER(Buffer) ASSERT UINTN) (Buffer)) & 0x01) == 0) +// +// Define the maximum debug and assert message length that this library supports +// +#define MAX_DEBUG_MESSAGE_LENGTH 0x100 + +#define ASCII_RED_ESC_SEQ "\033[31m" +#define
Re: [edk2-devel] [PATCH 1/1] MdePkg: Use ANSI colors to indicate debug message severity
Hi Rebecca, I think this is a good idea to improve readability. However, I think there is an assumption today that debug message output is just ASCII text with no assumptions on a terminal type and no assumption on screen width or screen height and no support for cursor control. This change would only make sense if the device the serial messages are sent supports ANSI color codes. I think we should make this a build time configuration option to add the ANSI sequences. I suggest a FeatureFlag PCD with a default of disabled to preserve current behavior and platforms can opt into ANSI color coded debug messages in their DSC file. Thanks, Mike > -Original Message- > From: Rebecca Cran > Sent: Monday, August 29, 2022 3:12 PM > To: devel@edk2.groups.io; Kinney, Michael D ; > Gao, Liming ; Liu, Zhiguang > > Cc: Rebecca Cran > Subject: [PATCH 1/1] MdePkg: Use ANSI colors to indicate debug message > severity > > There currently isn't a way to differentiate the different > levels of DEBUG output: DEBUG_ERROR, DEBUG_WARN, DEBUG_INFO > etc. > > To improve this, wrap DEBUG_ERROR and DEBUG_WARN level > messages in ANSI color code escape sequences. DEBUG_ERROR > messages will be displayed in red text, and DEBUG_WARN > in orange (the escape code is for the yellow color palette > entry, but it gets displayed as orange). > > Signed-off-by: Rebecca Cran > --- > MdePkg/Include/Library/PrintLib.h| 39 ++ > MdePkg/Library/BaseDebugLibSerialPort/DebugLib.c | 2 + > MdePkg/Library/BasePrintLib/PrintLib.c | 128 > MdePkg/Library/UefiDebugLibConOut/DebugLib.c | 2 + > 4 files changed, 171 insertions(+) > > diff --git a/MdePkg/Include/Library/PrintLib.h > b/MdePkg/Include/Library/PrintLib.h > index 8d523cac528d..06d9761df897 100644 > --- a/MdePkg/Include/Library/PrintLib.h > +++ b/MdePkg/Include/Library/PrintLib.h > @@ -2,6 +2,7 @@ >Provides services to print a formatted string to a buffer. All > combinations of >Unicode and ASCII strings are supported. > > +Copyright (c) 2022, Qualcomm Innovation Center, Inc. All rights reserved. > Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved. > SPDX-License-Identifier: BSD-2-Clause-Patent > > @@ -931,4 +932,42 @@ SPrintLengthAsciiFormat ( >IN VA_LIST Marker >); > > +/** > + Wraps a message with ANSI color escape codes. > + > + @param String The string to wrap. > + @param StringLen The size of the String buffer in characters. > + @param ErrorLevel The error level. > + > + @retval RETURN_SUCCESS The string was successfully updated. > + @retval RETURN_BUFFER_TOO_SMALL The buffer is too small. > + > +**/ > +RETURN_STATUS > +EFIAPI > +AsciiDebugGetColorString ( > + IN OUT CHAR8 *String, > + IN UINTN StringLen, > + IN UINTN ErrorLevel > + ); > + > +/** > + Wraps a message with ANSI color escape codes. > + > + @param String The string to wrap. > + @param StringLen The size of the String buffer in Unicode characters. > + @param ErrorLevel The error level. > + > + @retval RETURN_SUCCESS The string was successfully updated. > + @retval RETURN_BUFFER_TOO_SMALL The buffer is too small. > + > +**/ > +RETURN_STATUS > +EFIAPI > +UnicodeDebugGetColorString ( > + IN OUT CHAR16 *String, > + IN UINTN StringLen, > + IN UINTN ErrorLevel > + ); > + > #endif > diff --git a/MdePkg/Library/BaseDebugLibSerialPort/DebugLib.c > b/MdePkg/Library/BaseDebugLibSerialPort/DebugLib.c > index bd5686947712..d599d16f5b7a 100644 > --- a/MdePkg/Library/BaseDebugLibSerialPort/DebugLib.c > +++ b/MdePkg/Library/BaseDebugLibSerialPort/DebugLib.c > @@ -125,6 +125,8 @@ DebugPrintMarker ( > AsciiBSPrint (Buffer, sizeof (Buffer), Format, BaseListMarker); >} > > + AsciiDebugGetColorString (Buffer, MAX_DEBUG_MESSAGE_LENGTH, ErrorLevel); > + >// >// Send the print string to a Serial Port >// > diff --git a/MdePkg/Library/BasePrintLib/PrintLib.c > b/MdePkg/Library/BasePrintLib/PrintLib.c > index e6f4042bb90b..95135d45aff9 100644 > --- a/MdePkg/Library/BasePrintLib/PrintLib.c > +++ b/MdePkg/Library/BasePrintLib/PrintLib.c > @@ -1,12 +1,15 @@ > /** @file >Base Print Library instance implementation. > > + Copyright (c) 2022, Qualcomm Innovation Center, Inc. All rights > reserved. >Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved. >Portions copyright (c) 2008 - 2009, Apple Inc. All rights reserved. >SPDX-License-Identifier: BSD-2-Clause-Patent > > **/ > > +#include > + > #include "PrintLibInternal.h" > > // > @@ -19,6 +22,19 @@ VA_LIST gNullVaList; > > #define ASSERT_UNICODE_BUFFER(Buffer) ASSERT UINTN) (Buffer)) & 0x01) > == 0) > > +// > +// Define the maximum debug and assert message length that this library > supports > +// > +#define MAX_DEBUG_MESSAGE_LENGTH 0x100 > + > +#define ASCII_RED_ESC_SEQ "\033[31m" > +#define ASCII_YELLOW_ESC_SEQ "\033[33m" >
[edk2-devel] [PATCH 1/1] MdePkg: Use ANSI colors to indicate debug message severity
There currently isn't a way to differentiate the different
levels of DEBUG output: DEBUG_ERROR, DEBUG_WARN, DEBUG_INFO
etc.
To improve this, wrap DEBUG_ERROR and DEBUG_WARN level
messages in ANSI color code escape sequences. DEBUG_ERROR
messages will be displayed in red text, and DEBUG_WARN
in orange (the escape code is for the yellow color palette
entry, but it gets displayed as orange).
Signed-off-by: Rebecca Cran
---
MdePkg/Include/Library/PrintLib.h| 39 ++
MdePkg/Library/BaseDebugLibSerialPort/DebugLib.c | 2 +
MdePkg/Library/BasePrintLib/PrintLib.c | 128
MdePkg/Library/UefiDebugLibConOut/DebugLib.c | 2 +
4 files changed, 171 insertions(+)
diff --git a/MdePkg/Include/Library/PrintLib.h
b/MdePkg/Include/Library/PrintLib.h
index 8d523cac528d..06d9761df897 100644
--- a/MdePkg/Include/Library/PrintLib.h
+++ b/MdePkg/Include/Library/PrintLib.h
@@ -2,6 +2,7 @@
Provides services to print a formatted string to a buffer. All combinations
of
Unicode and ASCII strings are supported.
+Copyright (c) 2022, Qualcomm Innovation Center, Inc. All rights reserved.
Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.
SPDX-License-Identifier: BSD-2-Clause-Patent
@@ -931,4 +932,42 @@ SPrintLengthAsciiFormat (
IN VA_LIST Marker
);
+/**
+ Wraps a message with ANSI color escape codes.
+
+ @param String The string to wrap.
+ @param StringLen The size of the String buffer in characters.
+ @param ErrorLevel The error level.
+
+ @retval RETURN_SUCCESS The string was successfully updated.
+ @retval RETURN_BUFFER_TOO_SMALL The buffer is too small.
+
+**/
+RETURN_STATUS
+EFIAPI
+AsciiDebugGetColorString (
+ IN OUT CHAR8 *String,
+ IN UINTN StringLen,
+ IN UINTN ErrorLevel
+ );
+
+/**
+ Wraps a message with ANSI color escape codes.
+
+ @param String The string to wrap.
+ @param StringLen The size of the String buffer in Unicode characters.
+ @param ErrorLevel The error level.
+
+ @retval RETURN_SUCCESS The string was successfully updated.
+ @retval RETURN_BUFFER_TOO_SMALL The buffer is too small.
+
+**/
+RETURN_STATUS
+EFIAPI
+UnicodeDebugGetColorString (
+ IN OUT CHAR16 *String,
+ IN UINTN StringLen,
+ IN UINTN ErrorLevel
+ );
+
#endif
diff --git a/MdePkg/Library/BaseDebugLibSerialPort/DebugLib.c
b/MdePkg/Library/BaseDebugLibSerialPort/DebugLib.c
index bd5686947712..d599d16f5b7a 100644
--- a/MdePkg/Library/BaseDebugLibSerialPort/DebugLib.c
+++ b/MdePkg/Library/BaseDebugLibSerialPort/DebugLib.c
@@ -125,6 +125,8 @@ DebugPrintMarker (
AsciiBSPrint (Buffer, sizeof (Buffer), Format, BaseListMarker);
}
+ AsciiDebugGetColorString (Buffer, MAX_DEBUG_MESSAGE_LENGTH, ErrorLevel);
+
//
// Send the print string to a Serial Port
//
diff --git a/MdePkg/Library/BasePrintLib/PrintLib.c
b/MdePkg/Library/BasePrintLib/PrintLib.c
index e6f4042bb90b..95135d45aff9 100644
--- a/MdePkg/Library/BasePrintLib/PrintLib.c
+++ b/MdePkg/Library/BasePrintLib/PrintLib.c
@@ -1,12 +1,15 @@
/** @file
Base Print Library instance implementation.
+ Copyright (c) 2022, Qualcomm Innovation Center, Inc. All rights reserved.
Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.
Portions copyright (c) 2008 - 2009, Apple Inc. All rights reserved.
SPDX-License-Identifier: BSD-2-Clause-Patent
**/
+#include
+
#include "PrintLibInternal.h"
//
@@ -19,6 +22,19 @@ VA_LIST gNullVaList;
#define ASSERT_UNICODE_BUFFER(Buffer) ASSERT UINTN) (Buffer)) & 0x01) ==
0)
+//
+// Define the maximum debug and assert message length that this library
supports
+//
+#define MAX_DEBUG_MESSAGE_LENGTH 0x100
+
+#define ASCII_RED_ESC_SEQ "\033[31m"
+#define ASCII_YELLOW_ESC_SEQ "\033[33m"
+#define ASCII_END_ESC_SEQ "\033[0m"
+
+#define RED_ESC_SEQ L"\033[31m"
+#define YELLOW_ESC_SEQ L"\033[33m"
+#define END_ESC_SEQ L"\033[0m"
+
/**
Produces a Null-terminated Unicode string in an output buffer based on
a Null-terminated Unicode format string and a VA_LIST argument list.
@@ -834,3 +850,115 @@ SPrintLengthAsciiFormat (
{
return BasePrintLibSPrintMarker (NULL, 0, OUTPUT_UNICODE |
COUNT_ONLY_NO_PRINT, (CHAR8 *)FormatString, Marker, NULL);
}
+
+/**
+ Wraps a message with ANSI color escape codes.
+
+ @param String The string to wrap.
+ @param StringLen The size of the String buffer in characters.
+ @param ErrorLevel The error level.
+
+ @retval RETURN_SUCCESS The string was successfully updated.
+ @retval RETURN_BUFFER_TOO_SMALL The buffer is too small.
+
+**/
+RETURN_STATUS
+EFIAPI
+AsciiDebugGetColorString (
+ IN OUT CHAR8 *String,
+ IN UINTN StringLen,
+ IN UINTN ErrorLevel
+ )
+{
+ CHAR8 Buffer[MAX_DEBUG_MESSAGE_LENGTH];
+ UINTN ReqBufferLen;
+
+ ReqBufferLen = AsciiStrLen (String) +
+ AsciiStrLen (ASCII_RED_ESC_SEQ) +
+ AsciiStrLen (ASCII_END_ESC_SEQ) +
