Re: [PATCH 1/1] MdePkg: Use ANSI colors to indicate debug message severity

["Rebecca Cran" <quic_rcran@quicinc.com>]

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 <rebecca@quicinc.com>
>> Sent: Monday, August 29, 2022 3:12 PM
>> To: devel@edk2.groups.io; Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming <gaoliming@byosoft.com.cn>; Liu, Zhiguang
>> <zhiguang.liu@intel.com>
>> Cc: Rebecca Cran <rebecca@quicinc.com>
>> 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 <rebecca@quicinc.com>
>> ---
>>   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.<BR>
>>   Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
>>   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.<BR>
>>     Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
>>     Portions copyright (c) 2008 - 2009, Apple Inc. All rights reserved.<BR>
>>     SPDX-License-Identifier: BSD-2-Clause-Patent
>>
>>   **/
>>
>> +#include <Library/BaseMemoryLib.h>
>> +
>>   #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) +
>> +                 1;
>> +
>> +  if (StringLen < ReqBufferLen) {
>> +    return RETURN_BUFFER_TOO_SMALL;
>> +  }
>> +
>> +  ZeroMem (Buffer, sizeof (Buffer));
>> +
>> +  switch (ErrorLevel) {
>> +    case DEBUG_WARN:
>> +      AsciiStrCpyS (Buffer, MAX_DEBUG_MESSAGE_LENGTH, ASCII_YELLOW_ESC_SEQ);
>> +      break;
>> +    case DEBUG_ERROR:
>> +      AsciiStrCpyS (Buffer, MAX_DEBUG_MESSAGE_LENGTH, ASCII_RED_ESC_SEQ);
>> +      break;
>> +  }
>> +
>> +  AsciiStrCatS (Buffer, MAX_DEBUG_MESSAGE_LENGTH, String);
>> +
>> +  switch (ErrorLevel) {
>> +    case DEBUG_WARN:
>> +    case DEBUG_ERROR:
>> +      AsciiStrCatS (Buffer, MAX_DEBUG_MESSAGE_LENGTH, ASCII_END_ESC_SEQ);
>> +      break;
>> +  }
>> +
>> +  AsciiStrCpyS (String, StringLen, Buffer);
>> +
>> +  return RETURN_SUCCESS;
>> +}
>> +
>> +/**
>> +  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
>> +  )
>> +{
>> +  CHAR16  Buffer[MAX_DEBUG_MESSAGE_LENGTH];
>> +  UINTN   ReqBufferLen;
>> +
>> +  ReqBufferLen = StrLen (String) +
>> +                 StrLen (RED_ESC_SEQ) +
>> +                 StrLen (END_ESC_SEQ) +
>> +                 1;
>> +
>> +  if (StringLen < ReqBufferLen) {
>> +    return RETURN_BUFFER_TOO_SMALL;
>> +  }
>> +
>> +  ZeroMem (Buffer, sizeof (Buffer));
>> +
>> +  switch (ErrorLevel) {
>> +    case DEBUG_WARN:
>> +      StrCpyS (Buffer, MAX_DEBUG_MESSAGE_LENGTH, YELLOW_ESC_SEQ);
>> +      break;
>> +    case DEBUG_ERROR:
>> +      StrCpyS (Buffer, MAX_DEBUG_MESSAGE_LENGTH, RED_ESC_SEQ);
>> +      break;
>> +  }
>> +
>> +  StrCatS (Buffer, MAX_DEBUG_MESSAGE_LENGTH, String);
>> +
>> +  switch (ErrorLevel) {
>> +    case DEBUG_WARN:
>> +    case DEBUG_ERROR:
>> +      StrCatS (Buffer, MAX_DEBUG_MESSAGE_LENGTH, END_ESC_SEQ);
>> +      break;
>> +  }
>> +
>> +  StrCpyS (String, StringLen, Buffer);
>> +
>> +  return RETURN_SUCCESS;
>> +}
>> diff --git a/MdePkg/Library/UefiDebugLibConOut/DebugLib.c b/MdePkg/Library/UefiDebugLibConOut/DebugLib.c
>> index 65c8dc2b4654..bc902b0e60c6 100644
>> --- a/MdePkg/Library/UefiDebugLibConOut/DebugLib.c
>> +++ b/MdePkg/Library/UefiDebugLibConOut/DebugLib.c
>> @@ -108,6 +108,8 @@ DebugPrintMarker (
>>         UnicodeBSPrintAsciiFormat (Buffer, sizeof (Buffer), Format, BaseListMarker);
>>       }
>>
>> +    UnicodeDebugGetColorString (Buffer, MAX_DEBUG_MESSAGE_LENGTH, ErrorLevel);
>> +
>>       //
>>       // Send the print string to the Console Output device
>>       //
>> --
>> 2.30.2