Re: [edk2-devel] [PATCH] MdeModulePkg/TerminalDxe: Enhance the arrow keys support

["Ni, Ray" <ray.ni@intel.com>]

Zhichao,
The four new modes you newly added are supported by putty. But I do not think Putty is the standard the terminal driver needs to follow. 
So,
1. can you please remove the "putty" from the comments (maybe code)?
2. can you please find the sources where each mode is defined and double check whether the change you make is following the mode definition? It avoids you make Putty happy but make other good-behavior terminal software unhappy.

Thanks,
Ray

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Gao, Zhichao
> Sent: Wednesday, September 25, 2019 5:09 AM
> To: 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>; Gao, Liming
> <liming.gao@intel.com>
> Subject: [edk2-devel] [PATCH] MdeModulePkg/TerminalDxe: Enhance the arrow keys support
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2219
> 
> Refer to
> https://www.ssh.com/ssh/putty/putty-manuals/0.68/Chapter4.html#config-appcursor
> In normal mode, arrow key would send ESC [A through to ESC [D.
> In application mode, arrow key would send ESC OA through to
> ESC OD.
> 
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Hao A Wu <hao.a.wu@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Signed-off-by: Zhichao Gao <zhichao.gao@intel.com>
> ---
>  .../Console/TerminalDxe/TerminalConIn.c       | 46 ++++++++++++++++---
>  1 file changed, 40 insertions(+), 6 deletions(-)
> 
> diff --git a/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConIn.c
> b/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConIn.c
> index ac31f27984..fdb1a0f1ef 100644
> --- a/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConIn.c
> +++ b/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConIn.c
> @@ -1402,10 +1402,7 @@ UnicodeToEfiKey (
>          continue;
>        }
> 
> -      if (UnicodeChar == 'O' && (TerminalDevice->TerminalType == TerminalTypeVt100 ||
> -                                 TerminalDevice->TerminalType == TerminalTypeTtyTerm ||
> -                                 TerminalDevice->TerminalType == TerminalTypeXtermR6 ||
> -                                 TerminalDevice->TerminalType == TerminalTypeVt100Plus)) {
> +      if (UnicodeChar == 'O') {
>          TerminalDevice->InputState |= INPUT_STATE_O;
>          TerminalDevice->ResetState = RESET_STATE_DEFAULT;
>          continue;
> @@ -1631,6 +1628,35 @@ UnicodeToEfiKey (
>          }
>        }
> 
> +      //
> +      // In Putty application mode, the arrow keys would send
> +      // ESC OA through to ESC OD
> +      //
> +      if (TerminalDevice->TerminalType == TerminalTypePcAnsi    ||
> +          TerminalDevice->TerminalType == TerminalTypeVt100     ||
> +          TerminalDevice->TerminalType == TerminalTypeVt100Plus ||
> +          TerminalDevice->TerminalType == TerminalTypeVtUtf8    ||
> +          TerminalDevice->TerminalType == TerminalTypeTtyTerm   ||
> +          TerminalDevice->TerminalType == TerminalTypeLinux     ||
> +          TerminalDevice->TerminalType == TerminalTypeXtermR6   ||
> +          TerminalDevice->TerminalType == TerminalTypeVt400     ||
> +          TerminalDevice->TerminalType == TerminalTypeSCO) {
> +        switch (UnicodeChar) {
> +        case 'A':
> +          Key.ScanCode = SCAN_UP;
> +          break;
> +        case 'B':
> +          Key.ScanCode = SCAN_DOWN;
> +          break;
> +        case 'C':
> +          Key.ScanCode = SCAN_RIGHT;
> +          break;
> +        case 'D':
> +          Key.ScanCode = SCAN_LEFT;
> +          break;
> +        }
> +      }
> +
>        if (Key.ScanCode != SCAN_NULL) {
>          Key.UnicodeChar = 0;
>          EfiKeyFiFoInsertOneKey (TerminalDevice, &Key);
> @@ -1668,10 +1694,18 @@ UnicodeToEfiKey (
> 
>        Key.ScanCode = SCAN_NULL;
> 
> +      //
> +      // In Putty normal mode, the arrow keys would send
> +      // ESC [A through to [D
> +      //
>        if (TerminalDevice->TerminalType == TerminalTypePcAnsi    ||
>            TerminalDevice->TerminalType == TerminalTypeVt100     ||
> -          TerminalDevice->TerminalType == TerminalTypeVtUtf8 ||
> -          TerminalDevice->TerminalType == TerminalTypeTtyTerm ||
> +          TerminalDevice->TerminalType == TerminalTypeVt100Plus ||
> +          TerminalDevice->TerminalType == TerminalTypeVtUtf8    ||
> +          TerminalDevice->TerminalType == TerminalTypeTtyTerm   ||
> +          TerminalDevice->TerminalType == TerminalTypeLinux     ||
> +          TerminalDevice->TerminalType == TerminalTypeXtermR6   ||
> +          TerminalDevice->TerminalType == TerminalTypeVt400     ||
>            TerminalDevice->TerminalType == TerminalTypeSCO) {
>          switch (UnicodeChar) {
>          case 'A':
> --
> 2.21.0.windows.1
> 
> 
>