From: "Gao, Zhichao" <zhichao.gao@intel.com>
To: Caden Kline <cadenkline9@gmail.com>,
"devel@edk2.groups.io" <devel@edk2.groups.io>
Cc: "Wang, Jian J" <jian.j.wang@intel.com>,
"Wu, Hao A" <hao.a.wu@intel.com>, "Ni, Ray" <ray.ni@intel.com>,
"Desimone, Nathaniel L" <nathaniel.l.desimone@intel.com>
Subject: Re: [PATCH v3 1/1] MdeModulePkg/Console: Improve encoding of box drawing characters
Date: Thu, 9 Sep 2021 03:28:12 +0000 [thread overview]
Message-ID: <BN9PR11MB5275913D9BE71D8E3E5C03F0F6D59@BN9PR11MB5275.namprd11.prod.outlook.com> (raw)
In-Reply-To: <20210823024416.377856-2-cadenkline9@gmail.com>
Hi Caden,
The patch doesn't address all the comment I point out. If you think I am incorrect, please feel free to point out. See below:
> -----Original Message-----
> From: Caden Kline <cadenkline9@gmail.com>
> Sent: Monday, August 23, 2021 10:44 AM
> To: devel@edk2.groups.io
> Cc: Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>;
> Gao, Zhichao <zhichao.gao@intel.com>; Ni, Ray <ray.ni@intel.com>;
> Desimone, Nathaniel L <nathaniel.l.desimone@intel.com>
> Subject: [PATCH v3 1/1] MdeModulePkg/Console: Improve encoding of box
> drawing characters
>
> Improved encoding of box drawing characters for different terminal types.
> This includes Dec special graphics mode and more utf8.
> Changes are made according to the below issue
>
> https://bugzilla.tianocore.org/show_bug.cgi?id=3580
>
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Hao A Wu <hao.a.wu@intel.com>
> Cc: Zhichao Gao <zhichao.gao@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Signed-off-by: Caden Kline <cadenkline9@gmail.com>
> Reviewed-by: Nate DeSimone <nathaniel.l.desimone@intel.com>
> ---
> MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h | 26 +-
> MdeModulePkg/Universal/Console/TerminalDxe/Ansi.c | 2 +-
> MdeModulePkg/Universal/Console/TerminalDxe/TerminalConOut.c | 295
> +++++++++++++++-----
> 3 files changed, 243 insertions(+), 80 deletions(-)
>
> diff --git a/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h
> b/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h
> index 360e58e84743..1eab439531dc 100644
> --- a/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h
> +++ b/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h
> @@ -122,7 +122,12 @@ typedef struct {
> EFI_SIMPLE_TEXT_INPUT_EX_PROTOCOL SimpleInputEx;
>
> LIST_ENTRY NotifyList;
>
> EFI_EVENT KeyNotifyProcessEvent;
>
> + BOOLEAN DecSpecialGraphicsMode;
>
> } TERMINAL_DEV;
>
> +//
>
> +// This the length the escape sequences for entering and exiting Dec Special
> Graphics Mode
>
> +//
>
> +#define LENGTH_DEC_ESCAPE 0x03
>
>
>
> #define INPUT_STATE_DEFAULT 0x00
>
> #define INPUT_STATE_ESC 0x01
>
> @@ -169,6 +174,7 @@ typedef struct {
> UINT16 Unicode;
>
> CHAR8 PcAnsi;
>
> CHAR8 Ascii;
>
> + CHAR8 DecSpecialGraphics;
>
> } UNICODE_TO_CHAR;
>
>
>
> //
>
> @@ -1367,20 +1373,22 @@ Utf8ToUnicode (
> /**
>
> Detects if a Unicode char is for Box Drawing text graphics.
>
>
>
> - @param Graphic Unicode char to test.
>
> - @param PcAnsi Optional pointer to return PCANSI equivalent of
>
> - Graphic.
>
> - @param Ascii Optional pointer to return ASCII equivalent of
>
> - Graphic.
>
> -
>
> - @retval TRUE If Graphic is a supported Unicode Box Drawing character.
>
> + @param Graphic Unicode char to test.
>
> + @param PcAnsi Optional pointer to return PCANSI equivalent of
>
> + Graphic.
>
> + @param Ascii Optional pointer to return ASCII equivalent of
>
> + Graphic.
>
> + @param DecSpecialGraphics Optional pointer to return Dec Special
> Graphics equivalent of
>
> + Graphic.
>
> + @retval TRUE If Graphic is a supported Unicode Box Drawing
> character.
>
>
>
> **/
>
> BOOLEAN
>
> TerminalIsValidTextGraphics (
>
> IN CHAR16 Graphic,
>
> - OUT CHAR8 *PcAnsi, OPTIONAL
>
> - OUT CHAR8 *Ascii OPTIONAL
>
> + OUT CHAR8 *PcAnsi, OPTIONAL
>
> + OUT CHAR8 *Ascii, OPTIONAL
>
> + OUT CHAR8 *DecSpecialGraphics OPTIONAL
>
> );
>
>
>
> /**
>
> diff --git a/MdeModulePkg/Universal/Console/TerminalDxe/Ansi.c
> b/MdeModulePkg/Universal/Console/TerminalDxe/Ansi.c
> index f117d90b9de3..5ae5a4f0212e 100644
> --- a/MdeModulePkg/Universal/Console/TerminalDxe/Ansi.c
> +++ b/MdeModulePkg/Universal/Console/TerminalDxe/Ansi.c
> @@ -63,7 +63,7 @@ AnsiTestString (
>
>
> if ( !(TerminalIsValidAscii (*WString) ||
>
> TerminalIsValidEfiCntlChar (*WString) ||
>
> - TerminalIsValidTextGraphics (*WString, &GraphicChar, NULL) )) {
>
> + TerminalIsValidTextGraphics (*WString, &GraphicChar, NULL, NULL) )) {
>
>
>
> return EFI_UNSUPPORTED;
>
> }
>
> diff --git
> a/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConOut.c
> b/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConOut.c
> index aae470e9562c..7b328162325e 100644
> --- a/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConOut.c
> +++ b/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConOut.c
> @@ -16,61 +16,59 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
> //
>
> //
>
> UNICODE_TO_CHAR UnicodeToPcAnsiOrAscii[] = {
>
> - { BOXDRAW_HORIZONTAL, 0xc4, L'-' },
>
> - { BOXDRAW_VERTICAL, 0xb3, L'|' },
>
> - { BOXDRAW_DOWN_RIGHT, 0xda, L'/' },
>
> - { BOXDRAW_DOWN_LEFT, 0xbf, L'\\' },
>
> - { BOXDRAW_UP_RIGHT, 0xc0, L'\\' },
>
> - { BOXDRAW_UP_LEFT, 0xd9, L'/' },
>
> - { BOXDRAW_VERTICAL_RIGHT, 0xc3, L'|' },
>
> - { BOXDRAW_VERTICAL_LEFT, 0xb4, L'|' },
>
> - { BOXDRAW_DOWN_HORIZONTAL, 0xc2, L'+' },
>
> - { BOXDRAW_UP_HORIZONTAL, 0xc1, L'+' },
>
> - { BOXDRAW_VERTICAL_HORIZONTAL, 0xc5, L'+' },
>
> - { BOXDRAW_DOUBLE_HORIZONTAL, 0xcd, L'-' },
>
> - { BOXDRAW_DOUBLE_VERTICAL, 0xba, L'|' },
>
> - { BOXDRAW_DOWN_RIGHT_DOUBLE, 0xd5, L'/' },
>
> - { BOXDRAW_DOWN_DOUBLE_RIGHT, 0xd6, L'/' },
>
> - { BOXDRAW_DOUBLE_DOWN_RIGHT, 0xc9, L'/' },
>
> - { BOXDRAW_DOWN_LEFT_DOUBLE, 0xb8, L'\\' },
>
> - { BOXDRAW_DOWN_DOUBLE_LEFT, 0xb7, L'\\' },
>
> - { BOXDRAW_DOUBLE_DOWN_LEFT, 0xbb, L'\\' },
>
> - { BOXDRAW_UP_RIGHT_DOUBLE, 0xd4, L'\\' },
>
> - { BOXDRAW_UP_DOUBLE_RIGHT, 0xd3, L'\\' },
>
> - { BOXDRAW_DOUBLE_UP_RIGHT, 0xc8, L'\\' },
>
> - { BOXDRAW_UP_LEFT_DOUBLE, 0xbe, L'/' },
>
> - { BOXDRAW_UP_DOUBLE_LEFT, 0xbd, L'/' },
>
> - { BOXDRAW_DOUBLE_UP_LEFT, 0xbc, L'/' },
>
> - { BOXDRAW_VERTICAL_RIGHT_DOUBLE, 0xc6, L'|' },
>
> - { BOXDRAW_VERTICAL_DOUBLE_RIGHT, 0xc7, L'|' },
>
> - { BOXDRAW_DOUBLE_VERTICAL_RIGHT, 0xcc, L'|' },
>
> - { BOXDRAW_VERTICAL_LEFT_DOUBLE, 0xb5, L'|' },
>
> - { BOXDRAW_VERTICAL_DOUBLE_LEFT, 0xb6, L'|' },
>
> - { BOXDRAW_DOUBLE_VERTICAL_LEFT, 0xb9, L'|' },
>
> - { BOXDRAW_DOWN_HORIZONTAL_DOUBLE, 0xd1, L'+' },
>
> - { BOXDRAW_DOWN_DOUBLE_HORIZONTAL, 0xd2, L'+' },
>
> - { BOXDRAW_DOUBLE_DOWN_HORIZONTAL, 0xcb, L'+' },
>
> - { BOXDRAW_UP_HORIZONTAL_DOUBLE, 0xcf, L'+' },
>
> - { BOXDRAW_UP_DOUBLE_HORIZONTAL, 0xd0, L'+' },
>
> - { BOXDRAW_DOUBLE_UP_HORIZONTAL, 0xca, L'+' },
>
> - { BOXDRAW_VERTICAL_HORIZONTAL_DOUBLE, 0xd8, L'+' },
>
> - { BOXDRAW_VERTICAL_DOUBLE_HORIZONTAL, 0xd7, L'+' },
>
> - { BOXDRAW_DOUBLE_VERTICAL_HORIZONTAL, 0xce, L'+' },
>
> + { BOXDRAW_HORIZONTAL, 0xc4, L'-', 0x71 },
>
> + { BOXDRAW_VERTICAL, 0xb3, L'|', 0x78 },
>
> + { BOXDRAW_DOWN_RIGHT, 0xda, L'/', 0x6c },
>
> + { BOXDRAW_DOWN_LEFT, 0xbf, L'\\', 0x6b },
>
> + { BOXDRAW_UP_RIGHT, 0xc0, L'\\', 0x6d },
>
> + { BOXDRAW_UP_LEFT, 0xd9, L'/', 0x6a },
>
> + { BOXDRAW_VERTICAL_RIGHT, 0xc3, L'|', 0x74 },
>
> + { BOXDRAW_VERTICAL_LEFT, 0xb4, L'|', 0x75 },
>
> + { BOXDRAW_DOWN_HORIZONTAL, 0xc2, L'+', 0x77 },
>
> + { BOXDRAW_UP_HORIZONTAL, 0xc1, L'+', 0x76 },
>
> + { BOXDRAW_VERTICAL_HORIZONTAL, 0xc5, L'+', 0x6e },
>
> + { BOXDRAW_DOUBLE_HORIZONTAL, 0xcd, L'-', 0x71 },
>
> + { BOXDRAW_DOUBLE_VERTICAL, 0xba, L'|', 0x78 },
>
> + { BOXDRAW_DOWN_RIGHT_DOUBLE, 0xd5, L'/', 0x6c },
>
> + { BOXDRAW_DOWN_DOUBLE_RIGHT, 0xd6, L'/', 0x6c },
>
> + { BOXDRAW_DOUBLE_DOWN_RIGHT, 0xc9, L'/', 0x6c },
>
> + { BOXDRAW_DOWN_LEFT_DOUBLE, 0xb8, L'\\', 0x6b },
>
> + { BOXDRAW_DOWN_DOUBLE_LEFT, 0xb7, L'\\', 0x6b },
>
> + { BOXDRAW_DOUBLE_DOWN_LEFT, 0xbb, L'\\', 0x6b },
>
> + { BOXDRAW_UP_RIGHT_DOUBLE, 0xd4, L'\\', 0x6d },
>
> + { BOXDRAW_UP_DOUBLE_RIGHT, 0xd3, L'\\', 0x6d },
>
> + { BOXDRAW_DOUBLE_UP_RIGHT, 0xc8, L'\\', 0x6d },
>
> + { BOXDRAW_UP_LEFT_DOUBLE, 0xbe, L'/', 0x6a },
>
> + { BOXDRAW_UP_DOUBLE_LEFT, 0xbd, L'/', 0x6a },
>
> + { BOXDRAW_DOUBLE_UP_LEFT, 0xbc, L'/', 0x6a },
>
> + { BOXDRAW_VERTICAL_RIGHT_DOUBLE, 0xc6, L'|', 0x74 },
>
> + { BOXDRAW_VERTICAL_DOUBLE_RIGHT, 0xc7, L'|', 0x74 },
>
> + { BOXDRAW_DOUBLE_VERTICAL_RIGHT, 0xcc, L'|', 0x74 },
>
> + { BOXDRAW_VERTICAL_LEFT_DOUBLE, 0xb5, L'|', 0x75 },
>
> + { BOXDRAW_VERTICAL_DOUBLE_LEFT, 0xb6, L'|', 0x75 },
>
> + { BOXDRAW_DOUBLE_VERTICAL_LEFT, 0xb9, L'|', 0x75 },
>
> + { BOXDRAW_DOWN_HORIZONTAL_DOUBLE, 0xd1, L'+', 0x77 },
>
> + { BOXDRAW_DOWN_DOUBLE_HORIZONTAL, 0xd2, L'+', 0x77 },
>
> + { BOXDRAW_DOUBLE_DOWN_HORIZONTAL, 0xcb, L'+', 0x77 },
>
> + { BOXDRAW_UP_HORIZONTAL_DOUBLE, 0xcf, L'+', 0x76 },
>
> + { BOXDRAW_UP_DOUBLE_HORIZONTAL, 0xd0, L'+', 0x76 },
>
> + { BOXDRAW_DOUBLE_UP_HORIZONTAL, 0xca, L'+', 0x76 },
>
> + { BOXDRAW_VERTICAL_HORIZONTAL_DOUBLE, 0xd8, L'+', 0x6e },
>
> + { BOXDRAW_VERTICAL_DOUBLE_HORIZONTAL, 0xd7, L'+', 0x6e },
>
> + { BOXDRAW_DOUBLE_VERTICAL_HORIZONTAL, 0xce, L'+', 0x6e },
>
>
>
> - { BLOCKELEMENT_FULL_BLOCK, 0xdb, L'*' },
>
> - { BLOCKELEMENT_LIGHT_SHADE, 0xb0, L'+' },
>
> + { BLOCKELEMENT_FULL_BLOCK, 0xdb, L'*', 0x61 },
>
> + { BLOCKELEMENT_LIGHT_SHADE, 0xb0, L'+', 0x61 },
>
>
>
> - { GEOMETRICSHAPE_UP_TRIANGLE, '^', L'^' },
>
> - { GEOMETRICSHAPE_RIGHT_TRIANGLE, '>', L'>' },
>
> - { GEOMETRICSHAPE_DOWN_TRIANGLE, 'v', L'v' },
>
> - { GEOMETRICSHAPE_LEFT_TRIANGLE, '<', L'<' },
>
> + { GEOMETRICSHAPE_UP_TRIANGLE, '^', L'^', L'^' },
>
> + { GEOMETRICSHAPE_RIGHT_TRIANGLE, '>', L'>', L'>' },
>
> + { GEOMETRICSHAPE_DOWN_TRIANGLE, 'v', L'v', L'v' },
>
> + { GEOMETRICSHAPE_LEFT_TRIANGLE, '<', L'<', L'<' },
>
>
>
> - { ARROW_LEFT, '<', L'<' },
>
> - { ARROW_UP, '^', L'^' },
>
> - { ARROW_RIGHT, '>', L'>' },
>
> - { ARROW_DOWN, 'v', L'v' },
>
> -
>
> - { 0x0000, 0x00, L'\0' }
>
> + { ARROW_LEFT, '<', L'<', L'<' },
>
> + { ARROW_UP, '^', L'^', L'^' },
>
> + { ARROW_RIGHT, '>', L'>', L'>' },
>
> + { ARROW_DOWN, 'v', L'v', L'v' },
>
> };
>
>
>
> CHAR16 mSetModeString[] = { ESC, '[', '=', '3', 'h', 0 };
>
> @@ -80,6 +78,8 @@ CHAR16 mSetCursorPositionString[] = { ESC, '[', '0', '0', ';',
> '0', '0', 'H', 0
> CHAR16 mCursorForwardString[] = { ESC, '[', '0', '0', 'C', 0 };
>
> CHAR16 mCursorBackwardString[] = { ESC, '[', '0', '0', 'D', 0 };
>
>
>
> +CHAR8 SetDecModeString[] = {ESC, 0x28, 0x30};
>
> +CHAR8 ExitDecModeString[] = {ESC, 0x28, 0x42};
>
> //
>
> // Body of the ConOut functions
>
> //
>
> @@ -183,16 +183,19 @@ TerminalConOutOutputString (
> EFI_STATUS Status;
>
> UINT8 ValidBytes;
>
> CHAR8 CrLfStr[2];
>
> + CHAR8 DecChar;
>
> + UINTN ModeSwitchLength;
>
> //
>
> // flag used to indicate whether condition happens which will cause
>
> // return EFI_WARN_UNKNOWN_GLYPH
>
> //
>
> BOOLEAN Warning;
>
>
>
> - ValidBytes = 0;
>
> - Warning = FALSE;
>
> - AsciiChar = 0;
>
> -
>
> + ValidBytes = 0;
>
> + Warning = FALSE;
>
> + AsciiChar = 0;
>
> + DecChar = 0;
>
> + ModeSwitchLength = LENGTH_DEC_ESCAPE;
>
> //
>
> // get Terminal device data structure pointer.
>
> //
>
> @@ -217,17 +220,104 @@ TerminalConOutOutputString (
> for (; *WString != CHAR_NULL; WString++) {
>
>
>
> switch (TerminalDevice->TerminalType) {
>
> -
>
> case TerminalTypePcAnsi:
>
> - case TerminalTypeVt100:
>
> - case TerminalTypeVt100Plus:
>
> - case TerminalTypeTtyTerm:
>
> - case TerminalTypeLinux:
>
> + if (!TerminalIsValidTextGraphics (*WString, &GraphicChar, &AsciiChar,
> NULL)) {
>
> + //
>
> + // If it's not a graphic character convert Unicode to ASCII.
>
> + //
>
> + GraphicChar = (CHAR8)*WString;
>
> +
>
> + if (!(TerminalIsValidAscii (GraphicChar) || TerminalIsValidEfiCntlChar
> (GraphicChar))) {
>
> + //
>
> + // when this driver use the OutputString to output control string,
>
> + // TerminalDevice->OutputEscChar is set to let the Esc char
>
> + // to be output to the terminal emulation software.
>
> + //
>
> + if ((GraphicChar == ESC) && TerminalDevice->OutputEscChar) {
>
> + GraphicChar = ESC;
>
> + } else {
>
> + GraphicChar = '?';
>
> + Warning = TRUE;
>
> + }
>
> + }
>
> +
>
> + AsciiChar = GraphicChar;
>From my point, the above assignment is useless. If you think same with me, please remove it.
>
> + }
>
> + Length = 1;
>
> + Status = TerminalDevice->SerialIo->Write (
>
> + TerminalDevice->SerialIo,
>
> + &Length,
>
> + &GraphicChar
>
> + );
>
> +
>
> + if (EFI_ERROR (Status)) {
>
> + goto OutputError;
>
> + }
>
> +
>
> + break;
>
> case TerminalTypeXtermR6:
>
> - case TerminalTypeVt400:
>
> case TerminalTypeSCO:
>
> + //
>
> + // Box graphics are split into 2 types simple and advanced.
>
> + // Simple are drawn with dec special graphics.
>
> + // Advanced are drawn with utf8.
>
> + // This checks for simple because they have a lower value than the
> advanced.
>
> + //
>
> + if (TerminalIsValidTextGraphics (*WString, NULL, NULL, &DecChar) &&
> *WString < BOXDRAW_DOUBLE_HORIZONTAL) {
>
> + if (!TerminalDevice->DecSpecialGraphicsMode) {
>
> + ModeSwitchLength = LENGTH_DEC_ESCAPE;
>
> + Status = TerminalDevice->SerialIo->Write (
>
> + TerminalDevice->SerialIo,
>
> + &ModeSwitchLength,
>
> + (UINT8 *)SetDecModeString
>
> + );
>
> + if (EFI_ERROR (Status)) {
>
> + goto OutputError;
>
> + }
>
> + TerminalDevice->DecSpecialGraphicsMode = TRUE;
>
> + }
>
>
>
> - if (!TerminalIsValidTextGraphics (*WString, &GraphicChar, &AsciiChar)) {
>
> + GraphicChar = DecChar;
>
> + Length = 1;
>
> + } else {
>
> + if (TerminalDevice->DecSpecialGraphicsMode) {
>
The ModeSwitchLength should be initialized. Because it value would be changed in the loop.
> + Status = TerminalDevice->SerialIo->Write (
>
> + TerminalDevice->SerialIo,
>
> + &ModeSwitchLength,
>
> + (UINT8 *)ExitDecModeString
>
> + );
>
> + if (EFI_ERROR (Status)) {
>
> + goto OutputError;
>
> + }
>
> +
>
> + TerminalDevice->DecSpecialGraphicsMode = FALSE;
>
> + }
>
> + UnicodeToUtf8 (*WString, &Utf8Char, &ValidBytes);
>
> + Length = ValidBytes;
>
> + }
>
> +
>
> + if (ValidBytes) {
>
> + Status = TerminalDevice->SerialIo->Write (
>
> + TerminalDevice->SerialIo,
>
> + &Length,
>
> + (UINT8 *)&Utf8Char
>
> + );
>
> + ValidBytes = 0;
>
> + } else {
>
> + Status = TerminalDevice->SerialIo->Write (
>
> + TerminalDevice->SerialIo,
>
> + &Length,
>
> + &GraphicChar
>
> + );
>
> + }
>
> + if (EFI_ERROR (Status)) {
>
> + goto OutputError;
>
> + }
>
> +
>
> + break;
>
> + case TerminalTypeVt100:
>
> + case TerminalTypeTtyTerm:
>
> + if (!TerminalIsValidTextGraphics (*WString, &GraphicChar, &AsciiChar,
> NULL)) {
>
> //
>
> // If it's not a graphic character convert Unicode to ASCII.
>
> //
>
> @@ -239,8 +329,8 @@ TerminalConOutOutputString (
> // TerminalDevice->OutputEscChar is set to let the Esc char
>
> // to be output to the terminal emulation software.
>
> //
>
> - if ((GraphicChar == 27) && TerminalDevice->OutputEscChar) {
>
> - GraphicChar = 27;
>
> + if ((GraphicChar == ESC) && TerminalDevice->OutputEscChar) {
>
> + GraphicChar = ESC;
>
> } else {
>
> GraphicChar = '?';
>
> Warning = TRUE;
>
> @@ -248,14 +338,73 @@ TerminalConOutOutputString (
> }
>
>
>
> AsciiChar = GraphicChar;
>
> -
>
> }
>
>
>
> - if (TerminalDevice->TerminalType != TerminalTypePcAnsi) {
>
> - GraphicChar = AsciiChar;
>
> + GraphicChar = AsciiChar;
>
> +
>
> + Length = 1;
>
> +
>
> + Status = TerminalDevice->SerialIo->Write (
>
> + TerminalDevice->SerialIo,
>
> + &Length,
>
> + &GraphicChar
>
> + );
>
> +
>
> + if (EFI_ERROR (Status)) {
>
> + goto OutputError;
>
> }
>
>
>
> + break;
Indent alignment.
>
> + case TerminalTypeVt100Plus:
>
> + case TerminalTypeVt400:
>
> Length = 1;
>
> + if (TerminalIsValidTextGraphics (*WString, NULL, NULL, &DecChar)) {
>
> + if (!TerminalDevice->DecSpecialGraphicsMode) {
The above check cause the BLOCKELEMENT_FULL_BLOCK to ARROW_DOWN characters to be write with switch string. Is that expected?
>
> + ModeSwitchLength = LENGTH_DEC_ESCAPE;
>
> + Status = TerminalDevice->SerialIo->Write (
>
> + TerminalDevice->SerialIo,
>
> + &ModeSwitchLength,
>
> + (UINT8 *)SetDecModeString
>
> + );
>
> + if (EFI_ERROR (Status)) {
>
> + goto OutputError;
>
> + }
It doesn't show on the email, but it has tailing whitespace in above line. Please use "python ./BaseTools/Scripts/PatchCheck.py -1" to check the coding issue.
Thanks,
Zhichao
>
> + TerminalDevice->DecSpecialGraphicsMode = TRUE;
>
> + }
>
> +
>
> + GraphicChar = DecChar;
>
> + } else {
>
> + if (TerminalDevice->DecSpecialGraphicsMode) {
>
> + ModeSwitchLength = LENGTH_DEC_ESCAPE;
>
> + Status = TerminalDevice->SerialIo->Write (
>
> + TerminalDevice->SerialIo,
>
> + &ModeSwitchLength,
>
> + (UINT8 *)ExitDecModeString
>
> + );
>
> +
>
> + if (EFI_ERROR (Status)) {
>
> + goto OutputError;
>
> + }
>
> +
>
> + TerminalDevice->DecSpecialGraphicsMode = FALSE;
>
> + }
>
> +
>
> + GraphicChar = (CHAR8)*WString;
>
> +
>
> + if (!(TerminalIsValidAscii (GraphicChar) || TerminalIsValidEfiCntlChar
> (GraphicChar))) {
>
> + //
>
> + // when this driver use the OutputString to output control string,
>
> + // TerminalDevice->OutputEscChar is set to let the Esc char
>
> + // to be output to the terminal emulation software.
>
> + //
>
> + if ((GraphicChar == ESC) && TerminalDevice->OutputEscChar) {
>
> + GraphicChar = ESC;
>
> + } else {
>
> + GraphicChar = '?';
>
> + Warning = TRUE;
>
> + }
>
> + }
>
> + }
>
>
>
> Status = TerminalDevice->SerialIo->Write (
>
> TerminalDevice->SerialIo,
>
> @@ -268,7 +417,7 @@ TerminalConOutOutputString (
> }
>
>
>
> break;
>
> -
>
> + case TerminalTypeLinux:
>
> case TerminalTypeVtUtf8:
>
> UnicodeToUtf8 (*WString, &Utf8Char, &ValidBytes);
>
> Length = ValidBytes;
>
> @@ -280,8 +429,10 @@ TerminalConOutOutputString (
> if (EFI_ERROR (Status)) {
>
> goto OutputError;
>
> }
>
> +
>
> break;
>
> }
>
> +
>
> //
>
> // Update cursor position.
>
> //
>
> @@ -875,7 +1026,8 @@ BOOLEAN
> TerminalIsValidTextGraphics (
>
> IN CHAR16 Graphic,
>
> OUT CHAR8 *PcAnsi, OPTIONAL
>
> - OUT CHAR8 *Ascii OPTIONAL
>
> + OUT CHAR8 *Ascii, OPTIONAL
>
> + OUT CHAR8 *DecSpecialGraphics OPTIONAL
>
> )
>
> {
>
> UNICODE_TO_CHAR *Table;
>
> @@ -897,6 +1049,9 @@ TerminalIsValidTextGraphics (
> if (Ascii != NULL) {
>
> *Ascii = Table->Ascii;
>
> }
>
> + if (DecSpecialGraphics != NULL){
>
> + *DecSpecialGraphics = Table->DecSpecialGraphics;
>
> + }
>
>
>
> return TRUE;
>
> }
>
> --
> 2.33.0
prev parent reply other threads:[~2021-09-09 3:28 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-08-23 2:44 [PATCH v3 0/1] MdeModulePkg/Console: Improve encoding of box drawing characters Caden Kline
2021-08-23 2:44 ` [PATCH v3 1/1] " Caden Kline
2021-08-26 21:53 ` Nate DeSimone
2021-09-09 3:28 ` Gao, Zhichao [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=BN9PR11MB5275913D9BE71D8E3E5C03F0F6D59@BN9PR11MB5275.namprd11.prod.outlook.com \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox