public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* How to Interpret ReadKeyStrokeEX Data
@ 2018-06-01 18:26 Jim.Dailey
  2018-06-04  3:00 ` Ni, Ruiyu
  2018-06-04 17:31 ` Rothman, Michael A
  0 siblings, 2 replies; 12+ messages in thread
From: Jim.Dailey @ 2018-06-01 18:26 UTC (permalink / raw)
  To: ruiyu.ni; +Cc: jaben.carsey, felixp, edk2-devel

(Subject changed)

I guess this is a question of UEFI spec interpretation.  In the Console
I/O Protocol description of Version 2.5 of the spec (page 456), it says:

    If the Scan Code is set to 0x00 then the Unicode character is
    valid and should be used.

To me that clearly says it all.  The shift modifier is a don't care when
the scan code is zero.  And, this change in the shell code seems to be a
violation of that statement.

However, I also see some confusing (and grammatically incorrect) text in
the description of the ReadKeyStrokeEx() function of the simple text in
protocol that I am guessing is related to this change (*emphasis* mine):

    When interpreting the data from this function, it should be
    noted that if a class of printable characters that are normally
    *adjusted* by shift modifiers (e.g. Shift Key + "f" key) would
    be presented solely as a KeyData.Key.UnicodeChar without the
    associated shift state.

What I think the spec is trying to say here is that for A-Z and a-z,
the consumer does NOT need to look at the shift state to tell "A" from
"a", for example, because the Unicode character will be either "A" or
"a" as appropriate.

I think saying this is unnecessary simply because the earlier statement
(If the Scan Code is set to 0x00 then the Unicode character is valid and
should be used.) covers this case.

Further, I believe this text applies only to the A-Z keys because their
corresponding characters are *adjusted* (to upper case) when the shift
key is pressed. That is, if you adjust "blue" to "BLUE", you have two
different appearances, but only one meaning.

However, a "3" is not *adjusted* to a "#"; they are totally different
characters with different meanings altogether. No C pre-processor would
be happy seeing: "3ifdef SYMBOL".

In any case, I see nothing gained by ignoring keys having a zero scan
code and a valid Unicode character; the spec says that "the Unicode
character is valid and should be used".

Regards,
Jim

> -----Original Message-----
> From: Ni, Ruiyu [mailto:ruiyu.ni@intel.com] 
> Sent: Thursday, May 31, 2018 7:19 PM
> To: Dailey, Jim
> Cc: Carsey, Jaben; felixp@mail.ru; edk2-devel@lists.01.org
> Subject: RE: [edk2] [PATCH] ShellPkg/[hex]edit: use SimpleTextInEx to read console
> 
> Can you check which keyboard driver are you using?
> The keyboard driver is expected to translate "SHIFT" + "3" to "#" (without Shift state).
> I know that some keyboard driver doesn't do that correctly.
> E.g.: SHIFT + "3" is translated to "#" but the SHIFT state is not masked off.
> 
> [Sorry I thought I sent the mail out days ago]
>
>> -----Original Message-----
>> From: Jim.Dailey@dell.com [mailto:Jim.Dailey@dell.com]
>> Sent: Wednesday, May 23, 2018 3:01 AM
>> To: Ni, Ruiyu <ruiyu.ni@intel.com>
>> Cc: Carsey, Jaben <jaben.carsey@intel.com>; felixp@mail.ru; edk2-
>> devel@lists.01.org
>> Subject: RE: [edk2] [PATCH] ShellPkg/[hex]edit: use SimpleTextInEx to read
>> console
>> 
>> Ray,
>> 
>> The patch in the message below was applied to the tree on 12 Feb this year
>> (5563281fa2b31093a1cbd415553b9264c5136e89).
>> 
>> Part of the change to MainTextEditor.c causes an issue where I cannot enter (at
>> least some) shifted punctuation.  For example, after this check in I cannot edit a
>> shell script and create a comment because I cannot enter the "#" character.
>> When I try to type "#", the status bar simply shows "Unknown Command".
>> 
>> I don't really understand the change, but if in the MainEditorKeyInput function in
>> file MainTextEditor.c I delete the "NoShiftState" check from the first "else if"
>> below:
>> 
>> +        //
>> +        // dispatch to different components' key handling function
>> +        //
>> +        if (EFI_NOT_FOUND != MenuBarDispatchControlHotKey(&KeyData)) {
>> +          Status = EFI_SUCCESS;
>> +        } else if (NoShiftState && ((KeyData.Key.ScanCode == SCAN_NULL) ||
>> ((KeyData.Key.ScanCode >= SCAN_UP) && (KeyData.Key.ScanCode <=
>> SCAN_PAGE_DOWN)))) {
>> +          Status = FileBufferHandleInput (&KeyData.Key);
>> +        } else if (NoShiftState && (KeyData.Key.ScanCode >= SCAN_F1) &&
>> (KeyData.Key.ScanCode <= SCAN_F12)) {
>> +          Status = MenuBarDispatchFunctionKey (&KeyData.Key);
>> +        } else {
>> +          StatusBarSetStatusString (L"Unknown Command");
>> +          FileBufferMouseNeedRefresh = FALSE;
>> +        }
>> 
>> then I am able to enter "#" and other characters that I previously was unable to
>> enter.
>> 
>> Can you have a look at this?  I assume any shell binary built with this change will
>> have a similar issue.
>> 
>> Thanks,
>> Jim
>> 
>>> -----Original Message-----
>>> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Ruiyu
>>> Ni
>>> Sent: Monday, February 12, 2018 9:34 AM
>>> To: edk2-devel@lists.01.org
>>> Cc: Jaben Carsey; Felix
>>> Subject: [edk2] [PATCH] ShellPkg/[hex]edit: use SimpleTextInEx to read console
>>> 
>>> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=682
>>> 
>>> Edit and HexEdit commands assume that SimpleTxtIn translates
>>> Ctrl+<Alpha-Key> key combinations into Unicode control characters
>>> (0x1-0x1A).
>>> 
>>> Such translation does not seem to be required by the UEFI spec.
>>> Shell should not rely on implementation specific behavior.
>>> It should instead use SimpleTextInEx to read Ctrl+<Alpha-Key> key combinations.
>>> 
>>> The patch changes edit and hexedit to only consumes SimpleTextInEx.
>>> 
>>> Contributed-under: TianoCore Contribution Agreement 1.1
>>> Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
>>> Reported-by: Felix <felixp@mail.ru>
>>> Cc: Felix <felixp@mail.ru>
>>> Cc: Jaben Carsey <jaben.carsey@intel.com>
>>> ---
>>>  .../Edit/MainTextEditor.c                          | 135 +++++++++-----
>>>  .../Edit/TextEditorTypes.h                         |  21 ++-
>>>  .../UefiShellDebug1CommandsLib/EditInputBar.c      |  34 +++-
>>>  .../UefiShellDebug1CommandsLib/EditInputBar.h      |   6 +-
>>>  .../UefiShellDebug1CommandsLib/EditMenuBar.c       |  38 +++-
>>>  .../UefiShellDebug1CommandsLib/EditMenuBar.h       |   6 +-
>>>  .../HexEdit/HexEditorTypes.h                       |  25 +--
>>>  .../HexEdit/MainHexEditor.c                        | 205 +++++++++++++--------
>>>  8 files changed, 309 insertions(+), 161 deletions(-)
>>> 
>>> ---------8<----- clip ----->8--------
>>> 
>>> --
>>> 2.16.1.windows.1
>>> 
>>> _______________________________________________
>>> edk2-devel mailing list
>>> edk2-devel@lists.01.org
>>> https://lists.01.org/mailman/listinfo/edk2-devel



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: How to Interpret ReadKeyStrokeEX Data
  2018-06-01 18:26 How to Interpret ReadKeyStrokeEX Data Jim.Dailey
@ 2018-06-04  3:00 ` Ni, Ruiyu
  2018-06-04 14:46   ` Jim.Dailey
  2018-06-04 17:31 ` Rothman, Michael A
  1 sibling, 1 reply; 12+ messages in thread
From: Ni, Ruiyu @ 2018-06-04  3:00 UTC (permalink / raw)
  To: Jim.Dailey@dell.com
  Cc: Carsey, Jaben, felixp@mail.ru, edk2-devel@lists.01.org



Thanks/Ray

> -----Original Message-----
> From: Jim.Dailey@dell.com <Jim.Dailey@dell.com>
> Sent: Saturday, June 2, 2018 2:27 AM
> To: Ni, Ruiyu <ruiyu.ni@intel.com>
> Cc: Carsey, Jaben <jaben.carsey@intel.com>; felixp@mail.ru; edk2-
> devel@lists.01.org
> Subject: How to Interpret ReadKeyStrokeEX Data
> 
> (Subject changed)
> 
> I guess this is a question of UEFI spec interpretation.  In the Console
> I/O Protocol description of Version 2.5 of the spec (page 456), it says:
> 
>     If the Scan Code is set to 0x00 then the Unicode character is
>     valid and should be used.
> 
> To me that clearly says it all.  The shift modifier is a don't care when
> the scan code is zero.  And, this change in the shell code seems to be a
> violation of that statement.
Considering there is the other protocol called SimpleTextIn which only returns
Scan Code and Unicode Character. The above statement only says that
consumer should only care one of the fields: Scan Code and Unicode Character.

> 
> However, I also see some confusing (and grammatically incorrect) text in
> the description of the ReadKeyStrokeEx() function of the simple text in
> protocol that I am guessing is related to this change (*emphasis* mine):
> 
>     When interpreting the data from this function, it should be
>     noted that if a class of printable characters that are normally
>     *adjusted* by shift modifiers (e.g. Shift Key + "f" key) would
>     be presented solely as a KeyData.Key.UnicodeChar without the
>     associated shift state.

Please also considering an implementation of SimpleTextIn, if "SHIFT + 3" is
pressed, what Unicode Character should be returned since there is no place
to return the shift state for SimpleTextIn.
I think it should return "#".

> 
> What I think the spec is trying to say here is that for A-Z and a-z,
> the consumer does NOT need to look at the shift state to tell "A" from
> "a", for example, because the Unicode character will be either "A" or
> "a" as appropriate.
> 
> I think saying this is unnecessary simply because the earlier statement
> (If the Scan Code is set to 0x00 then the Unicode character is valid and
> should be used.) covers this case.
> 
> Further, I believe this text applies only to the A-Z keys because their
> corresponding characters are *adjusted* (to upper case) when the shift
> key is pressed. That is, if you adjust "blue" to "BLUE", you have two
> different appearances, but only one meaning.
> 
> However, a "3" is not *adjusted* to a "#"; they are totally different
> characters with different meanings altogether. No C pre-processor would
> be happy seeing: "3ifdef SYMBOL".
> 
> In any case, I see nothing gained by ignoring keys having a zero scan
> code and a valid Unicode character; the spec says that "the Unicode
> character is valid and should be used".
> 
> Regards,
> Jim
> 
> > -----Original Message-----
> > From: Ni, Ruiyu [mailto:ruiyu.ni@intel.com]
> > Sent: Thursday, May 31, 2018 7:19 PM
> > To: Dailey, Jim
> > Cc: Carsey, Jaben; felixp@mail.ru; edk2-devel@lists.01.org
> > Subject: RE: [edk2] [PATCH] ShellPkg/[hex]edit: use SimpleTextInEx to read
> console
> >
> > Can you check which keyboard driver are you using?
> > The keyboard driver is expected to translate "SHIFT" + "3" to "#" (without
> Shift state).
> > I know that some keyboard driver doesn't do that correctly.
> > E.g.: SHIFT + "3" is translated to "#" but the SHIFT state is not masked off.
> >
> > [Sorry I thought I sent the mail out days ago]
> >
> >> -----Original Message-----
> >> From: Jim.Dailey@dell.com [mailto:Jim.Dailey@dell.com]
> >> Sent: Wednesday, May 23, 2018 3:01 AM
> >> To: Ni, Ruiyu <ruiyu.ni@intel.com>
> >> Cc: Carsey, Jaben <jaben.carsey@intel.com>; felixp@mail.ru; edk2-
> >> devel@lists.01.org
> >> Subject: RE: [edk2] [PATCH] ShellPkg/[hex]edit: use SimpleTextInEx to
> read
> >> console
> >>
> >> Ray,
> >>
> >> The patch in the message below was applied to the tree on 12 Feb this
> year
> >> (5563281fa2b31093a1cbd415553b9264c5136e89).
> >>
> >> Part of the change to MainTextEditor.c causes an issue where I cannot
> enter (at
> >> least some) shifted punctuation.  For example, after this check in I cannot
> edit a
> >> shell script and create a comment because I cannot enter the "#"
> character.
> >> When I try to type "#", the status bar simply shows "Unknown Command".
> >>
> >> I don't really understand the change, but if in the MainEditorKeyInput
> function in
> >> file MainTextEditor.c I delete the "NoShiftState" check from the first "else
> if"
> >> below:
> >>
> >> +        //
> >> +        // dispatch to different components' key handling function
> >> +        //
> >> +        if (EFI_NOT_FOUND != MenuBarDispatchControlHotKey(&KeyData))
> {
> >> +          Status = EFI_SUCCESS;
> >> +        } else if (NoShiftState && ((KeyData.Key.ScanCode == SCAN_NULL)
> ||
> >> ((KeyData.Key.ScanCode >= SCAN_UP) && (KeyData.Key.ScanCode <=
> >> SCAN_PAGE_DOWN)))) {
> >> +          Status = FileBufferHandleInput (&KeyData.Key);
> >> +        } else if (NoShiftState && (KeyData.Key.ScanCode >= SCAN_F1) &&
> >> (KeyData.Key.ScanCode <= SCAN_F12)) {
> >> +          Status = MenuBarDispatchFunctionKey (&KeyData.Key);
> >> +        } else {
> >> +          StatusBarSetStatusString (L"Unknown Command");
> >> +          FileBufferMouseNeedRefresh = FALSE;
> >> +        }
> >>
> >> then I am able to enter "#" and other characters that I previously was
> unable to
> >> enter.
> >>
> >> Can you have a look at this?  I assume any shell binary built with this
> change will
> >> have a similar issue.
> >>
> >> Thanks,
> >> Jim
> >>
> >>> -----Original Message-----
> >>> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf
> Of Ruiyu
> >>> Ni
> >>> Sent: Monday, February 12, 2018 9:34 AM
> >>> To: edk2-devel@lists.01.org
> >>> Cc: Jaben Carsey; Felix
> >>> Subject: [edk2] [PATCH] ShellPkg/[hex]edit: use SimpleTextInEx to read
> console
> >>>
> >>> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=682
> >>>
> >>> Edit and HexEdit commands assume that SimpleTxtIn translates
> >>> Ctrl+<Alpha-Key> key combinations into Unicode control characters
> >>> (0x1-0x1A).
> >>>
> >>> Such translation does not seem to be required by the UEFI spec.
> >>> Shell should not rely on implementation specific behavior.
> >>> It should instead use SimpleTextInEx to read Ctrl+<Alpha-Key> key
> combinations.
> >>>
> >>> The patch changes edit and hexedit to only consumes SimpleTextInEx.
> >>>
> >>> Contributed-under: TianoCore Contribution Agreement 1.1
> >>> Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
> >>> Reported-by: Felix <felixp@mail.ru>
> >>> Cc: Felix <felixp@mail.ru>
> >>> Cc: Jaben Carsey <jaben.carsey@intel.com>
> >>> ---
> >>>  .../Edit/MainTextEditor.c                          | 135 +++++++++-----
> >>>  .../Edit/TextEditorTypes.h                         |  21 ++-
> >>>  .../UefiShellDebug1CommandsLib/EditInputBar.c      |  34 +++-
> >>>  .../UefiShellDebug1CommandsLib/EditInputBar.h      |   6 +-
> >>>  .../UefiShellDebug1CommandsLib/EditMenuBar.c       |  38 +++-
> >>>  .../UefiShellDebug1CommandsLib/EditMenuBar.h       |   6 +-
> >>>  .../HexEdit/HexEditorTypes.h                       |  25 +--
> >>>  .../HexEdit/MainHexEditor.c                        | 205 +++++++++++++--------
> >>>  8 files changed, 309 insertions(+), 161 deletions(-)
> >>>
> >>> ---------8<----- clip ----->8--------
> >>>
> >>> --
> >>> 2.16.1.windows.1
> >>>
> >>> _______________________________________________
> >>> edk2-devel mailing list
> >>> edk2-devel@lists.01.org
> >>> https://lists.01.org/mailman/listinfo/edk2-devel



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: How to Interpret ReadKeyStrokeEX Data
  2018-06-04  3:00 ` Ni, Ruiyu
@ 2018-06-04 14:46   ` Jim.Dailey
  2018-06-04 14:49     ` Andrew Fish
  0 siblings, 1 reply; 12+ messages in thread
From: Jim.Dailey @ 2018-06-04 14:46 UTC (permalink / raw)
  To: ruiyu.ni; +Cc: jaben.carsey, edk2-devel, felixp


>Thanks/Ray
>
>> From: Jim.Dailey@dell.com <Jim.Dailey@dell.com>
>> 
>> I guess this is a question of UEFI spec interpretation.  In the Console
>> I/O Protocol description of Version 2.5 of the spec (page 456), it says:
>> 
>>     If the Scan Code is set to 0x00 then the Unicode character is
>>     valid and should be used.
>> 
>> To me that clearly says it all.  The shift modifier is a don't care when
>> the scan code is zero.  And, this change in the shell code seems to be a
>> violation of that statement.
>Considering there is the other protocol called SimpleTextIn which only returns
>Scan Code and Unicode Character. The above statement only says that
>consumer should only care one of the fields: Scan Code and Unicode Character.

The editor is not using that protocol, but it matters not because your
question is my point exactly: when the scan code is zero, nothing else
matters except the Unicode character.

>> However, I also see some confusing (and grammatically incorrect) text in
>> the description of the ReadKeyStrokeEx() function of the simple text in
>> protocol that I am guessing is related to this change (*emphasis* mine):
>> 
>>     When interpreting the data from this function, it should be
>>     noted that if a class of printable characters that are normally
>>     *adjusted* by shift modifiers (e.g. Shift Key + "f" key) would
>>     be presented solely as a KeyData.Key.UnicodeChar without the
>>     associated shift state.
>
>Please also considering an implementation of SimpleTextIn, if "SHIFT + 3" is
>pressed, what Unicode Character should be returned since there is no place
>to return the shift state for SimpleTextIn.
>I think it should return "#".

Again the editor does not use that protocol (if it did, I don't think
we'd even be having this discussion!).  But to answer your question, of
course it should return "#".  There is no question about that. And, when
it returns "#", it would be wrong to ignore that for any reason.

The editor is using the SimpleTextInEx protocol which adds information
about the shift state that SimpleTextIn does not supply.

The question I am raising is when SimpleTextInEx returns, for example:

   Scan Code         = 0
   Unicode Char      = 0x0023 ("#")
   Shift Information = 0x80000001 (right shift pressed)

is it correct for the editor to reject this as an invalid key?

I say, no, it would be wrong to reject this data because the scan code
is 0 and, therefore, the Unicode character is valid and should be used.

The change made back on February 12 says, yes, because the shift data
is greater than 0x80000000 (i.e. shift state is valid and some "shift"
key was pressed).

>>> Can you check which keyboard driver are you using?
>>> The keyboard driver is expected to translate "SHIFT" + "3" to "#" (without
>>> Shift state).
>>> I know that some keyboard driver doesn't do that correctly.
>>> E.g.: SHIFT + "3" is translated to "#" but the SHIFT state is not masked off.

As far as I can tell I am using the standard MdeModulePkg USB driver.



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: How to Interpret ReadKeyStrokeEX Data
  2018-06-04 14:46   ` Jim.Dailey
@ 2018-06-04 14:49     ` Andrew Fish
  2018-06-04 16:00       ` Jim.Dailey
  0 siblings, 1 reply; 12+ messages in thread
From: Andrew Fish @ 2018-06-04 14:49 UTC (permalink / raw)
  To: Jim.Dailey; +Cc: ruiyu.ni, jaben.carsey, edk2-devel, felixp

Jim,

The big picture difference is the original SimpleTextIn was the least common denominator with a serial terminal. The Ex version added more info about keyboards, so richer info on modifier keys. 

Thanks,

Andrew Fish

> On Jun 4, 2018, at 7:46 AM, Jim.Dailey@dell.com wrote:
> 
> 
>> Thanks/Ray
>> 
>>> From: Jim.Dailey@dell.com <Jim.Dailey@dell.com>
>>> 
>>> I guess this is a question of UEFI spec interpretation.  In the Console
>>> I/O Protocol description of Version 2.5 of the spec (page 456), it says:
>>> 
>>>    If the Scan Code is set to 0x00 then the Unicode character is
>>>    valid and should be used.
>>> 
>>> To me that clearly says it all.  The shift modifier is a don't care when
>>> the scan code is zero.  And, this change in the shell code seems to be a
>>> violation of that statement.
>> Considering there is the other protocol called SimpleTextIn which only returns
>> Scan Code and Unicode Character. The above statement only says that
>> consumer should only care one of the fields: Scan Code and Unicode Character.
> 
> The editor is not using that protocol, but it matters not because your
> question is my point exactly: when the scan code is zero, nothing else
> matters except the Unicode character.
> 
>>> However, I also see some confusing (and grammatically incorrect) text in
>>> the description of the ReadKeyStrokeEx() function of the simple text in
>>> protocol that I am guessing is related to this change (*emphasis* mine):
>>> 
>>>    When interpreting the data from this function, it should be
>>>    noted that if a class of printable characters that are normally
>>>    *adjusted* by shift modifiers (e.g. Shift Key + "f" key) would
>>>    be presented solely as a KeyData.Key.UnicodeChar without the
>>>    associated shift state.
>> 
>> Please also considering an implementation of SimpleTextIn, if "SHIFT + 3" is
>> pressed, what Unicode Character should be returned since there is no place
>> to return the shift state for SimpleTextIn.
>> I think it should return "#".
> 
> Again the editor does not use that protocol (if it did, I don't think
> we'd even be having this discussion!).  But to answer your question, of
> course it should return "#".  There is no question about that. And, when
> it returns "#", it would be wrong to ignore that for any reason.
> 
> The editor is using the SimpleTextInEx protocol which adds information
> about the shift state that SimpleTextIn does not supply.
> 
> The question I am raising is when SimpleTextInEx returns, for example:
> 
>   Scan Code         = 0
>   Unicode Char      = 0x0023 ("#")
>   Shift Information = 0x80000001 (right shift pressed)
> 
> is it correct for the editor to reject this as an invalid key?
> 
> I say, no, it would be wrong to reject this data because the scan code
> is 0 and, therefore, the Unicode character is valid and should be used.
> 
> The change made back on February 12 says, yes, because the shift data
> is greater than 0x80000000 (i.e. shift state is valid and some "shift"
> key was pressed).
> 
>>>> Can you check which keyboard driver are you using?
>>>> The keyboard driver is expected to translate "SHIFT" + "3" to "#" (without
>>>> Shift state).
>>>> I know that some keyboard driver doesn't do that correctly.
>>>> E.g.: SHIFT + "3" is translated to "#" but the SHIFT state is not masked off.
> 
> As far as I can tell I am using the standard MdeModulePkg USB driver.
> 
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: How to Interpret ReadKeyStrokeEX Data
  2018-06-04 14:49     ` Andrew Fish
@ 2018-06-04 16:00       ` Jim.Dailey
  2018-06-04 16:02         ` Jim.Dailey
  0 siblings, 1 reply; 12+ messages in thread
From: Jim.Dailey @ 2018-06-04 16:00 UTC (permalink / raw)
  To: afish; +Cc: ruiyu.ni, jaben.carsey, edk2-devel, felixp

Dell - Internal Use - Confidential  

> From: afish@apple.com [mailto:afish@apple.com] 
>
> The big picture difference is the original SimpleTextIn was the least common
> denominator with a serial terminal. The Ex version added more info about
> keyboards, so richer info on modifier keys.

I get that.  But I fail to see how that affects SimpleTextInEx behavior or
what the UEFI spec has to say about it.

As I said earlier, the question I am raising is when SimpleTextInEx returns
something like:
 
   Scan Code         = 0
   Unicode Char      = 0x0023 ("#")
   Shift Information = 0x80000001 (right shift pressed)
 
is it correct for the editor to reject this as an invalid key?

I say, no, it would be wrong to reject this data because the scan code
is 0 and, therefore, the Unicode character is valid and should be used.


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: How to Interpret ReadKeyStrokeEX Data
  2018-06-04 16:00       ` Jim.Dailey
@ 2018-06-04 16:02         ` Jim.Dailey
  2018-06-04 16:21           ` Carsey, Jaben
  0 siblings, 1 reply; 12+ messages in thread
From: Jim.Dailey @ 2018-06-04 16:02 UTC (permalink / raw)
  To: afish; +Cc: ruiyu.ni, jaben.carsey, edk2-devel, felixp

Please disregard the stupid "Confidential" line that our email tool
adds but hides from me when I send text. :-(

-----Original Message-----
From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Dailey, Jim
Sent: Monday, June 4, 2018 11:00 AM
To: afish@apple.com
Cc: ruiyu.ni@intel.com; jaben.carsey@intel.com; edk2-devel@lists.01.org; felixp@mail.ru
Subject: Re: [edk2] How to Interpret ReadKeyStrokeEX Data

Dell - Internal Use - Confidential   <=== THIS IS BOGUS !!

> From: afish@apple.com [mailto:afish@apple.com] 
>
> The big picture difference is the original SimpleTextIn was the least common
> denominator with a serial terminal. The Ex version added more info about
> keyboards, so richer info on modifier keys.

I get that.  But I fail to see how that affects SimpleTextInEx behavior or
what the UEFI spec has to say about it.

As I said earlier, the question I am raising is when SimpleTextInEx returns
something like:
 
   Scan Code         = 0
   Unicode Char      = 0x0023 ("#")
   Shift Information = 0x80000001 (right shift pressed)
 
is it correct for the editor to reject this as an invalid key?

I say, no, it would be wrong to reject this data because the scan code
is 0 and, therefore, the Unicode character is valid and should be used.
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: How to Interpret ReadKeyStrokeEX Data
  2018-06-04 16:02         ` Jim.Dailey
@ 2018-06-04 16:21           ` Carsey, Jaben
  2018-06-04 16:28             ` Jim.Dailey
  0 siblings, 1 reply; 12+ messages in thread
From: Carsey, Jaben @ 2018-06-04 16:21 UTC (permalink / raw)
  To: Jim.Dailey@dell.com
  Cc: afish@apple.com, Ni, Ruiyu, edk2-devel@lists.01.org,
	felixp@mail.ru, Rothman, Michael A

Jim,

I think I see what you mean.  If scan code is 0, then don’t test the shift state and just use the character raw?

Jaben
> On Jun 4, 2018, at 9:02 AM, "Jim.Dailey@dell.com" <Jim.Dailey@dell.com> wrote:
> 
> Please disregard the stupid "Confidential" line that our email tool
> adds but hides from me when I send text. :-(
> 
> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Dailey, Jim
> Sent: Monday, June 4, 2018 11:00 AM
> To: afish@apple.com
> Cc: ruiyu.ni@intel.com; jaben.carsey@intel.com; edk2-devel@lists.01.org; felixp@mail.ru
> Subject: Re: [edk2] How to Interpret ReadKeyStrokeEX Data
> 
> Dell - Internal Use - Confidential   <=== THIS IS BOGUS !!
> 
>> From: afish@apple.com [mailto:afish@apple.com] 
>> 
>> The big picture difference is the original SimpleTextIn was the least common
>> denominator with a serial terminal. The Ex version added more info about
>> keyboards, so richer info on modifier keys.
> 
> I get that.  But I fail to see how that affects SimpleTextInEx behavior or
> what the UEFI spec has to say about it.
> 
> As I said earlier, the question I am raising is when SimpleTextInEx returns
> something like:
> 
>   Scan Code         = 0
>   Unicode Char      = 0x0023 ("#")
>   Shift Information = 0x80000001 (right shift pressed)
> 
> is it correct for the editor to reject this as an invalid key?
> 
> I say, no, it would be wrong to reject this data because the scan code
> is 0 and, therefore, the Unicode character is valid and should be used.
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: How to Interpret ReadKeyStrokeEX Data
  2018-06-04 16:21           ` Carsey, Jaben
@ 2018-06-04 16:28             ` Jim.Dailey
  0 siblings, 0 replies; 12+ messages in thread
From: Jim.Dailey @ 2018-06-04 16:28 UTC (permalink / raw)
  To: jaben.carsey; +Cc: afish, ruiyu.ni, edk2-devel, felixp, michael.a.rothman

Yes!  Sorry if I was unclear.

-----Original Message-----
From: Carsey, Jaben [mailto:jaben.carsey@intel.com] 
Sent: Monday, June 4, 2018 11:22 AM
To: Dailey, Jim
Cc: afish@apple.com; Ni, Ruiyu; edk2-devel@lists.01.org; felixp@mail.ru; Rothman, Michael A
Subject: Re: [edk2] How to Interpret ReadKeyStrokeEX Data

Jim,

I think I see what you mean.  If scan code is 0, then don't test the shift state and just use the character raw?

Jaben
> On Jun 4, 2018, at 9:02 AM, "Jim.Dailey@dell.com" <Jim.Dailey@dell.com> wrote:
> 
> Please disregard the stupid "Confidential" line that our email tool
> adds but hides from me when I send text. :-(
> 
> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Dailey, Jim
> Sent: Monday, June 4, 2018 11:00 AM
> To: afish@apple.com
> Cc: ruiyu.ni@intel.com; jaben.carsey@intel.com; edk2-devel@lists.01.org; felixp@mail.ru
> Subject: Re: [edk2] How to Interpret ReadKeyStrokeEX Data
> 
> Dell - Internal Use - Confidential   <=== THIS IS BOGUS !!
> 
>> From: afish@apple.com [mailto:afish@apple.com] 
>> 
>> The big picture difference is the original SimpleTextIn was the least common
>> denominator with a serial terminal. The Ex version added more info about
>> keyboards, so richer info on modifier keys.
> 
> I get that.  But I fail to see how that affects SimpleTextInEx behavior or
> what the UEFI spec has to say about it.
> 
> As I said earlier, the question I am raising is when SimpleTextInEx returns
> something like:
> 
>   Scan Code         = 0
>   Unicode Char      = 0x0023 ("#")
>   Shift Information = 0x80000001 (right shift pressed)
> 
> is it correct for the editor to reject this as an invalid key?
> 
> I say, no, it would be wrong to reject this data because the scan code
> is 0 and, therefore, the Unicode character is valid and should be used.
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: How to Interpret ReadKeyStrokeEX Data
  2018-06-01 18:26 How to Interpret ReadKeyStrokeEX Data Jim.Dailey
  2018-06-04  3:00 ` Ni, Ruiyu
@ 2018-06-04 17:31 ` Rothman, Michael A
  2018-06-06  3:37   ` Rothman, Michael A
  1 sibling, 1 reply; 12+ messages in thread
From: Rothman, Michael A @ 2018-06-04 17:31 UTC (permalink / raw)
  To: Jim.Dailey@dell.com, Ni, Ruiyu
  Cc: Carsey, Jaben, edk2-devel@lists.01.org, felixp@mail.ru

Since I'm largely the person who might be to blame for the language and intent here and I’ll focus on the spec-related item.  See my comments below.



Thanks,

Mike Rothman

(迈克 罗斯曼 / माइकल रोथ्मेन् / Михаил Ротман / משה רוטמן)

רועה עיקרי של חתולים





-----Original Message-----
From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Jim.Dailey@dell.com
Sent: Friday, June 1, 2018 11:27 AM
To: Ni, Ruiyu <ruiyu.ni@intel.com>
Cc: Carsey, Jaben <jaben.carsey@intel.com>; edk2-devel@lists.01.org; felixp@mail.ru
Subject: [edk2] How to Interpret ReadKeyStrokeEX Data



(Subject changed)



I guess this is a question of UEFI spec interpretation.  In the Console I/O Protocol description of Version 2.5 of the spec (page 456), it says:



    If the Scan Code is set to 0x00 then the Unicode character is

    valid and should be used.



To me that clearly says it all.  The shift modifier is a don't care when the scan code is zero.  And, this change in the shell code seems to be a violation of that statement.



However, I also see some confusing (and grammatically incorrect) text in the description of the ReadKeyStrokeEx() function of the simple text in protocol that I am guessing is related to this change (*emphasis* mine):



    When interpreting the data from this function, it should be

    noted that if a class of printable characters that are normally

    *adjusted* by shift modifiers (e.g. Shift Key + "f" key) would

    be presented solely as a KeyData.Key.UnicodeChar without the

    associated shift state.



What I think the spec is trying to say here is that for A-Z and a-z, the consumer does NOT need to look at the shift state to tell "A" from "a", for example, because the Unicode character will be either "A" or "a" as appropriate.



n  No, it is any key that would create a displayable character that adjusts based on the shift state. Realize that the users of ReadKeyStroke/ReadKeyStrokeEx fall back to a common denominator of Scan Code or Unicode Character. So if someone types a shift 4, the underlying keyboard layout that the keyboard driver controls would dictate how that gets translated. On my keyboard in the US it turns into a “$” symbol, while someone in Europe may very well have a software-defined keyboard layout which translates the same keystroke to a “€” symbol. That of course applies to the many characters you specified (A-F, a-f) and many others.

n  The text above is intended to imply what it says, “a class of printable characters … would be presented solely as a KeyData.Key.UnicodeChar without the associated shift state. This makes consumers of both the Ex and Non-Ex variant of ReadKeyStroke able to use the same logic to test for any shiftable characters by simply looking at the Unicode value. I’d note the shift and toggle states (which are only available on Ex) are there not so much for interpreting the translated key but to maximize flexibility associated with keyboard mapping as a UEFI application.



I think saying this is unnecessary simply because the earlier statement (If the Scan Code is set to 0x00 then the Unicode character is valid and should be used.) covers this case.



Further, I believe this text applies only to the A-Z keys because their corresponding characters are *adjusted* (to upper case) when the shift key is pressed. That is, if you adjust "blue" to "BLUE", you have two different appearances, but only one meaning.



However, a "3" is not *adjusted* to a "#"; they are totally different characters with different meanings altogether. No C pre-processor would be happy seeing: "3ifdef SYMBOL".



In any case, I see nothing gained by ignoring keys having a zero scan code and a valid Unicode character; the spec says that "the Unicode character is valid and should be used".



Regards,

Jim



> -----Original Message-----

> From: Ni, Ruiyu [mailto:ruiyu.ni@intel.com]

> Sent: Thursday, May 31, 2018 7:19 PM

> To: Dailey, Jim

> Cc: Carsey, Jaben; felixp@mail.ru<mailto:felixp@mail.ru>; edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>

> Subject: RE: [edk2] [PATCH] ShellPkg/[hex]edit: use SimpleTextInEx to

> read console

>

> Can you check which keyboard driver are you using?

> The keyboard driver is expected to translate "SHIFT" + "3" to "#" (without Shift state).

> I know that some keyboard driver doesn't do that correctly.

> E.g.: SHIFT + "3" is translated to "#" but the SHIFT state is not masked off.

>

> [Sorry I thought I sent the mail out days ago]

>

>> -----Original Message-----

>> From: Jim.Dailey@dell.com<mailto:Jim.Dailey@dell.com> [mailto:Jim.Dailey@dell.com]

>> Sent: Wednesday, May 23, 2018 3:01 AM

>> To: Ni, Ruiyu <ruiyu.ni@intel.com<mailto:ruiyu.ni@intel.com>>

>> Cc: Carsey, Jaben <jaben.carsey@intel.com<mailto:jaben.carsey@intel.com>>; felixp@mail.ru<mailto:felixp@mail.ru>; edk2-

>> devel@lists.01.org<mailto:devel@lists.01.org>

>> Subject: RE: [edk2] [PATCH] ShellPkg/[hex]edit: use SimpleTextInEx to

>> read console

>>

>> Ray,

>>

>> The patch in the message below was applied to the tree on 12 Feb this

>> year (5563281fa2b31093a1cbd415553b9264c5136e89).

>>

>> Part of the change to MainTextEditor.c causes an issue where I cannot

>> enter (at least some) shifted punctuation.  For example, after this

>> check in I cannot edit a shell script and create a comment because I cannot enter the "#" character.

>> When I try to type "#", the status bar simply shows "Unknown Command".

>>

>> I don't really understand the change, but if in the

>> MainEditorKeyInput function in file MainTextEditor.c I delete the "NoShiftState" check from the first "else if"

>> below:

>>

>> +        //

>> +        // dispatch to different components' key handling function

>> +        //

>> +        if (EFI_NOT_FOUND != MenuBarDispatchControlHotKey(&KeyData)) {

>> +          Status = EFI_SUCCESS;

>> +        } else if (NoShiftState && ((KeyData.Key.ScanCode ==

>> + SCAN_NULL) ||

>> ((KeyData.Key.ScanCode >= SCAN_UP) && (KeyData.Key.ScanCode <=

>> SCAN_PAGE_DOWN)))) {

>> +          Status = FileBufferHandleInput (&KeyData.Key);

>> +        } else if (NoShiftState && (KeyData.Key.ScanCode >= SCAN_F1)

>> + &&

>> (KeyData.Key.ScanCode <= SCAN_F12)) {

>> +          Status = MenuBarDispatchFunctionKey (&KeyData.Key);

>> +        } else {

>> +          StatusBarSetStatusString (L"Unknown Command");

>> +          FileBufferMouseNeedRefresh = FALSE;

>> +        }

>>

>> then I am able to enter "#" and other characters that I previously

>> was unable to enter.

>>

>> Can you have a look at this?  I assume any shell binary built with

>> this change will have a similar issue.

>>

>> Thanks,

>> Jim

>>

>>> -----Original Message-----

>>> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf

>>> Of Ruiyu Ni

>>> Sent: Monday, February 12, 2018 9:34 AM

>>> To: edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>

>>> Cc: Jaben Carsey; Felix

>>> Subject: [edk2] [PATCH] ShellPkg/[hex]edit: use SimpleTextInEx to

>>> read console

>>>

>>> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=682

>>>

>>> Edit and HexEdit commands assume that SimpleTxtIn translates

>>> Ctrl+<Alpha-Key> key combinations into Unicode control characters

>>> (0x1-0x1A).

>>>

>>> Such translation does not seem to be required by the UEFI spec.

>>> Shell should not rely on implementation specific behavior.

>>> It should instead use SimpleTextInEx to read Ctrl+<Alpha-Key> key combinations.

>>>

>>> The patch changes edit and hexedit to only consumes SimpleTextInEx.

>>>

>>> Contributed-under: TianoCore Contribution Agreement 1.1

>>> Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com<mailto:ruiyu.ni@intel.com>>

>>> Reported-by: Felix <felixp@mail.ru<mailto:felixp@mail.ru>>

>>> Cc: Felix <felixp@mail.ru<mailto:felixp@mail.ru>>

>>> Cc: Jaben Carsey <jaben.carsey@intel.com<mailto:jaben.carsey@intel.com>>

>>> ---

>>>  .../Edit/MainTextEditor.c                          | 135 +++++++++-----

>>>  .../Edit/TextEditorTypes.h                         |  21 ++-

>>>  .../UefiShellDebug1CommandsLib/EditInputBar.c      |  34 +++-

>>>  .../UefiShellDebug1CommandsLib/EditInputBar.h      |   6 +-

>>>  .../UefiShellDebug1CommandsLib/EditMenuBar.c       |  38 +++-

>>>  .../UefiShellDebug1CommandsLib/EditMenuBar.h       |   6 +-

>>>  .../HexEdit/HexEditorTypes.h                       |  25 +--

>>>  .../HexEdit/MainHexEditor.c                        | 205 +++++++++++++--------

>>>  8 files changed, 309 insertions(+), 161 deletions(-)

>>>

>>> ---------8<----- clip ----->8--------

>>>

>>> --

>>> 2.16.1.windows.1

>>>

>>> _______________________________________________

>>> edk2-devel mailing list

>>> edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>

>>> https://lists.01.org/mailman/listinfo/edk2-devel



_______________________________________________

edk2-devel mailing list

edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>

https://lists.01.org/mailman/listinfo/edk2-devel

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: How to Interpret ReadKeyStrokeEX Data
  2018-06-04 17:31 ` Rothman, Michael A
@ 2018-06-06  3:37   ` Rothman, Michael A
  2018-06-06 13:03     ` Jim.Dailey
  0 siblings, 1 reply; 12+ messages in thread
From: Rothman, Michael A @ 2018-06-06  3:37 UTC (permalink / raw)
  To: Rothman, Michael A, Jim.Dailey@dell.com, Ni, Ruiyu
  Cc: Carsey, Jaben, edk2-devel@lists.01.org, felixp@mail.ru

Jim,

I think the problem you're seeing is that the USB keyboard driver you're using is downrev and needs to be updated.

If you look at https://github.com/tianocore/edk2/commit/dd190645eb43424706eb1709d0032c69a1935d9f there was a fix checked in to address exactly the issue you're running into. It's basically a symptom of running a new shell without a correspondingly updated keyboard driver. The new shell in effect exposed a latent bug.

Hope that helps.

Thanks,
Mike Rothman 
(迈克 罗斯曼 / माइकल रोथ्मेन् / Михаил Ротман / משה רוטמן)
רועה עיקרי של חתולים

-----Original Message-----
From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Rothman, Michael A
Sent: Monday, June 4, 2018 10:31 AM
To: Jim.Dailey@dell.com; Ni, Ruiyu <ruiyu.ni@intel.com>
Cc: Carsey, Jaben <jaben.carsey@intel.com>; edk2-devel@lists.01.org; felixp@mail.ru
Subject: Re: [edk2] How to Interpret ReadKeyStrokeEX Data

Since I'm largely the person who might be to blame for the language and intent here and I’ll focus on the spec-related item.  See my comments below.



Thanks,

Mike Rothman

(迈克 罗斯曼 / माइकल रोथ्मेन् / Михаил Ротман / משה רוטמן)

רועה עיקרי של חתולים





-----Original Message-----
From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Jim.Dailey@dell.com
Sent: Friday, June 1, 2018 11:27 AM
To: Ni, Ruiyu <ruiyu.ni@intel.com>
Cc: Carsey, Jaben <jaben.carsey@intel.com>; edk2-devel@lists.01.org; felixp@mail.ru
Subject: [edk2] How to Interpret ReadKeyStrokeEX Data



(Subject changed)



I guess this is a question of UEFI spec interpretation.  In the Console I/O Protocol description of Version 2.5 of the spec (page 456), it says:



    If the Scan Code is set to 0x00 then the Unicode character is

    valid and should be used.



To me that clearly says it all.  The shift modifier is a don't care when the scan code is zero.  And, this change in the shell code seems to be a violation of that statement.



However, I also see some confusing (and grammatically incorrect) text in the description of the ReadKeyStrokeEx() function of the simple text in protocol that I am guessing is related to this change (*emphasis* mine):



    When interpreting the data from this function, it should be

    noted that if a class of printable characters that are normally

    *adjusted* by shift modifiers (e.g. Shift Key + "f" key) would

    be presented solely as a KeyData.Key.UnicodeChar without the

    associated shift state.



What I think the spec is trying to say here is that for A-Z and a-z, the consumer does NOT need to look at the shift state to tell "A" from "a", for example, because the Unicode character will be either "A" or "a" as appropriate.



n  No, it is any key that would create a displayable character that adjusts based on the shift state. Realize that the users of ReadKeyStroke/ReadKeyStrokeEx fall back to a common denominator of Scan Code or Unicode Character. So if someone types a shift 4, the underlying keyboard layout that the keyboard driver controls would dictate how that gets translated. On my keyboard in the US it turns into a “$” symbol, while someone in Europe may very well have a software-defined keyboard layout which translates the same keystroke to a “€” symbol. That of course applies to the many characters you specified (A-F, a-f) and many others.

n  The text above is intended to imply what it says, “a class of printable characters … would be presented solely as a KeyData.Key.UnicodeChar without the associated shift state. This makes consumers of both the Ex and Non-Ex variant of ReadKeyStroke able to use the same logic to test for any shiftable characters by simply looking at the Unicode value. I’d note the shift and toggle states (which are only available on Ex) are there not so much for interpreting the translated key but to maximize flexibility associated with keyboard mapping as a UEFI application.



I think saying this is unnecessary simply because the earlier statement (If the Scan Code is set to 0x00 then the Unicode character is valid and should be used.) covers this case.



Further, I believe this text applies only to the A-Z keys because their corresponding characters are *adjusted* (to upper case) when the shift key is pressed. That is, if you adjust "blue" to "BLUE", you have two different appearances, but only one meaning.



However, a "3" is not *adjusted* to a "#"; they are totally different characters with different meanings altogether. No C pre-processor would be happy seeing: "3ifdef SYMBOL".



In any case, I see nothing gained by ignoring keys having a zero scan code and a valid Unicode character; the spec says that "the Unicode character is valid and should be used".



Regards,

Jim



> -----Original Message-----

> From: Ni, Ruiyu [mailto:ruiyu.ni@intel.com]

> Sent: Thursday, May 31, 2018 7:19 PM

> To: Dailey, Jim

> Cc: Carsey, Jaben; felixp@mail.ru<mailto:felixp@mail.ru>; edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>

> Subject: RE: [edk2] [PATCH] ShellPkg/[hex]edit: use SimpleTextInEx to

> read console

>

> Can you check which keyboard driver are you using?

> The keyboard driver is expected to translate "SHIFT" + "3" to "#" (without Shift state).

> I know that some keyboard driver doesn't do that correctly.

> E.g.: SHIFT + "3" is translated to "#" but the SHIFT state is not masked off.

>

> [Sorry I thought I sent the mail out days ago]

>

>> -----Original Message-----

>> From: Jim.Dailey@dell.com<mailto:Jim.Dailey@dell.com> [mailto:Jim.Dailey@dell.com]

>> Sent: Wednesday, May 23, 2018 3:01 AM

>> To: Ni, Ruiyu <ruiyu.ni@intel.com<mailto:ruiyu.ni@intel.com>>

>> Cc: Carsey, Jaben <jaben.carsey@intel.com<mailto:jaben.carsey@intel.com>>; felixp@mail.ru<mailto:felixp@mail.ru>; edk2-

>> devel@lists.01.org<mailto:devel@lists.01.org>

>> Subject: RE: [edk2] [PATCH] ShellPkg/[hex]edit: use SimpleTextInEx to

>> read console

>>

>> Ray,

>>

>> The patch in the message below was applied to the tree on 12 Feb this

>> year (5563281fa2b31093a1cbd415553b9264c5136e89).

>>

>> Part of the change to MainTextEditor.c causes an issue where I cannot

>> enter (at least some) shifted punctuation.  For example, after this

>> check in I cannot edit a shell script and create a comment because I cannot enter the "#" character.

>> When I try to type "#", the status bar simply shows "Unknown Command".

>>

>> I don't really understand the change, but if in the

>> MainEditorKeyInput function in file MainTextEditor.c I delete the "NoShiftState" check from the first "else if"

>> below:

>>

>> +        //

>> +        // dispatch to different components' key handling function

>> +        //

>> +        if (EFI_NOT_FOUND != MenuBarDispatchControlHotKey(&KeyData)) {

>> +          Status = EFI_SUCCESS;

>> +        } else if (NoShiftState && ((KeyData.Key.ScanCode ==

>> + SCAN_NULL) ||

>> ((KeyData.Key.ScanCode >= SCAN_UP) && (KeyData.Key.ScanCode <=

>> SCAN_PAGE_DOWN)))) {

>> +          Status = FileBufferHandleInput (&KeyData.Key);

>> +        } else if (NoShiftState && (KeyData.Key.ScanCode >= SCAN_F1)

>> + &&

>> (KeyData.Key.ScanCode <= SCAN_F12)) {

>> +          Status = MenuBarDispatchFunctionKey (&KeyData.Key);

>> +        } else {

>> +          StatusBarSetStatusString (L"Unknown Command");

>> +          FileBufferMouseNeedRefresh = FALSE;

>> +        }

>>

>> then I am able to enter "#" and other characters that I previously

>> was unable to enter.

>>

>> Can you have a look at this?  I assume any shell binary built with

>> this change will have a similar issue.

>>

>> Thanks,

>> Jim

>>

>>> -----Original Message-----

>>> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf

>>> Of Ruiyu Ni

>>> Sent: Monday, February 12, 2018 9:34 AM

>>> To: edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>

>>> Cc: Jaben Carsey; Felix

>>> Subject: [edk2] [PATCH] ShellPkg/[hex]edit: use SimpleTextInEx to

>>> read console

>>>

>>> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=682

>>>

>>> Edit and HexEdit commands assume that SimpleTxtIn translates

>>> Ctrl+<Alpha-Key> key combinations into Unicode control characters

>>> (0x1-0x1A).

>>>

>>> Such translation does not seem to be required by the UEFI spec.

>>> Shell should not rely on implementation specific behavior.

>>> It should instead use SimpleTextInEx to read Ctrl+<Alpha-Key> key combinations.

>>>

>>> The patch changes edit and hexedit to only consumes SimpleTextInEx.

>>>

>>> Contributed-under: TianoCore Contribution Agreement 1.1

>>> Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com<mailto:ruiyu.ni@intel.com>>

>>> Reported-by: Felix <felixp@mail.ru<mailto:felixp@mail.ru>>

>>> Cc: Felix <felixp@mail.ru<mailto:felixp@mail.ru>>

>>> Cc: Jaben Carsey <jaben.carsey@intel.com<mailto:jaben.carsey@intel.com>>

>>> ---

>>>  .../Edit/MainTextEditor.c                          | 135 +++++++++-----

>>>  .../Edit/TextEditorTypes.h                         |  21 ++-

>>>  .../UefiShellDebug1CommandsLib/EditInputBar.c      |  34 +++-

>>>  .../UefiShellDebug1CommandsLib/EditInputBar.h      |   6 +-

>>>  .../UefiShellDebug1CommandsLib/EditMenuBar.c       |  38 +++-

>>>  .../UefiShellDebug1CommandsLib/EditMenuBar.h       |   6 +-

>>>  .../HexEdit/HexEditorTypes.h                       |  25 +--

>>>  .../HexEdit/MainHexEditor.c                        | 205 +++++++++++++--------

>>>  8 files changed, 309 insertions(+), 161 deletions(-)

>>>

>>> ---------8<----- clip ----->8--------

>>>

>>> --

>>> 2.16.1.windows.1

>>>

>>> _______________________________________________

>>> edk2-devel mailing list

>>> edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>

>>> https://lists.01.org/mailman/listinfo/edk2-devel



_______________________________________________

edk2-devel mailing list

edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>

https://lists.01.org/mailman/listinfo/edk2-devel
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: How to Interpret ReadKeyStrokeEX Data
  2018-06-06  3:37   ` Rothman, Michael A
@ 2018-06-06 13:03     ` Jim.Dailey
  2018-06-06 13:30       ` Andrew Fish
  0 siblings, 1 reply; 12+ messages in thread
From: Jim.Dailey @ 2018-06-06 13:03 UTC (permalink / raw)
  To: michael.a.rothman; +Cc: jaben.carsey, edk2-devel, felixp, ruiyu.ni

Thanks, Mike.  I figured that out yesterday.

I think the only problem now is that any shell built with the original change to edit will not work as expected if it is run on a system whose BIOS was created before the second change was made (i.e. pretty much every existing UEFI BIOS in the world!).

Since the shell is a stand-alone tool that is not necessarily tied to any particular machine, I think that it is likely that more than a few people may run into this problem in the future.

Regards,
Jim

-----Original Message-----
From: Rothman, Michael A [mailto:michael.a.rothman@intel.com] 
Sent: Tuesday, June 5, 2018 10:38 PM
To: Rothman, Michael A; Dailey, Jim; Ni, Ruiyu
Cc: Carsey, Jaben; edk2-devel@lists.01.org; felixp@mail.ru
Subject: RE: How to Interpret ReadKeyStrokeEX Data

Jim,

I think the problem you're seeing is that the USB keyboard driver you're using is downrev and needs to be updated.

If you look at https://github.com/tianocore/edk2/commit/dd190645eb43424706eb1709d0032c69a1935d9f there was a fix checked in to address exactly the issue you're running into. It's basically a symptom of running a new shell without a correspondingly updated keyboard driver. The new shell in effect exposed a latent bug.

Hope that helps.

Thanks,
Mike Rothman 
(迈克 罗斯曼 / माइकल रोथ्मेन् / Михаил Ротман / משה רוטמן)
רועה עיקרי של חתולים

-----Original Message-----
From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Rothman, Michael A
Sent: Monday, June 4, 2018 10:31 AM
To: Jim.Dailey@dell.com; Ni, Ruiyu <ruiyu.ni@intel.com>
Cc: Carsey, Jaben <jaben.carsey@intel.com>; edk2-devel@lists.01.org; felixp@mail.ru
Subject: Re: [edk2] How to Interpret ReadKeyStrokeEX Data

Since I'm largely the person who might be to blame for the language and intent here and I’ll focus on the spec-related item.  See my comments below.



Thanks,

Mike Rothman

(迈克 罗斯曼 / माइकल रोथ्मेन् / Михаил Ротман / משה רוטמן)

רועה עיקרי של חתולים





-----Original Message-----
From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Jim.Dailey@dell.com
Sent: Friday, June 1, 2018 11:27 AM
To: Ni, Ruiyu <ruiyu.ni@intel.com>
Cc: Carsey, Jaben <jaben.carsey@intel.com>; edk2-devel@lists.01.org; felixp@mail.ru
Subject: [edk2] How to Interpret ReadKeyStrokeEX Data



(Subject changed)



I guess this is a question of UEFI spec interpretation.  In the Console I/O Protocol description of Version 2.5 of the spec (page 456), it says:



    If the Scan Code is set to 0x00 then the Unicode character is

    valid and should be used.



To me that clearly says it all.  The shift modifier is a don't care when the scan code is zero.  And, this change in the shell code seems to be a violation of that statement.



However, I also see some confusing (and grammatically incorrect) text in the description of the ReadKeyStrokeEx() function of the simple text in protocol that I am guessing is related to this change (*emphasis* mine):



    When interpreting the data from this function, it should be

    noted that if a class of printable characters that are normally

    *adjusted* by shift modifiers (e.g. Shift Key + "f" key) would

    be presented solely as a KeyData.Key.UnicodeChar without the

    associated shift state.



What I think the spec is trying to say here is that for A-Z and a-z, the consumer does NOT need to look at the shift state to tell "A" from "a", for example, because the Unicode character will be either "A" or "a" as appropriate.



n  No, it is any key that would create a displayable character that adjusts based on the shift state. Realize that the users of ReadKeyStroke/ReadKeyStrokeEx fall back to a common denominator of Scan Code or Unicode Character. So if someone types a shift 4, the underlying keyboard layout that the keyboard driver controls would dictate how that gets translated. On my keyboard in the US it turns into a “$” symbol, while someone in Europe may very well have a software-defined keyboard layout which translates the same keystroke to a “€” symbol. That of course applies to the many characters you specified (A-F, a-f) and many others.

n  The text above is intended to imply what it says, “a class of printable characters … would be presented solely as a KeyData.Key.UnicodeChar without the associated shift state. This makes consumers of both the Ex and Non-Ex variant of ReadKeyStroke able to use the same logic to test for any shiftable characters by simply looking at the Unicode value. I’d note the shift and toggle states (which are only available on Ex) are there not so much for interpreting the translated key but to maximize flexibility associated with keyboard mapping as a UEFI application.



I think saying this is unnecessary simply because the earlier statement (If the Scan Code is set to 0x00 then the Unicode character is valid and should be used.) covers this case.



Further, I believe this text applies only to the A-Z keys because their corresponding characters are *adjusted* (to upper case) when the shift key is pressed. That is, if you adjust "blue" to "BLUE", you have two different appearances, but only one meaning.



However, a "3" is not *adjusted* to a "#"; they are totally different characters with different meanings altogether. No C pre-processor would be happy seeing: "3ifdef SYMBOL".



In any case, I see nothing gained by ignoring keys having a zero scan code and a valid Unicode character; the spec says that "the Unicode character is valid and should be used".



Regards,

Jim



> -----Original Message-----

> From: Ni, Ruiyu [mailto:ruiyu.ni@intel.com]

> Sent: Thursday, May 31, 2018 7:19 PM

> To: Dailey, Jim

> Cc: Carsey, Jaben; felixp@mail.ru<mailto:felixp@mail.ru>; edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>

> Subject: RE: [edk2] [PATCH] ShellPkg/[hex]edit: use SimpleTextInEx to

> read console

>

> Can you check which keyboard driver are you using?

> The keyboard driver is expected to translate "SHIFT" + "3" to "#" (without Shift state).

> I know that some keyboard driver doesn't do that correctly.

> E.g.: SHIFT + "3" is translated to "#" but the SHIFT state is not masked off.

>

> [Sorry I thought I sent the mail out days ago]

>

>> -----Original Message-----

>> From: Jim.Dailey@dell.com<mailto:Jim.Dailey@dell.com> [mailto:Jim.Dailey@dell.com]

>> Sent: Wednesday, May 23, 2018 3:01 AM

>> To: Ni, Ruiyu <ruiyu.ni@intel.com<mailto:ruiyu.ni@intel.com>>

>> Cc: Carsey, Jaben <jaben.carsey@intel.com<mailto:jaben.carsey@intel.com>>; felixp@mail.ru<mailto:felixp@mail.ru>; edk2-

>> devel@lists.01.org<mailto:devel@lists.01.org>

>> Subject: RE: [edk2] [PATCH] ShellPkg/[hex]edit: use SimpleTextInEx to

>> read console

>>

>> Ray,

>>

>> The patch in the message below was applied to the tree on 12 Feb this

>> year (5563281fa2b31093a1cbd415553b9264c5136e89).

>>

>> Part of the change to MainTextEditor.c causes an issue where I cannot

>> enter (at least some) shifted punctuation.  For example, after this

>> check in I cannot edit a shell script and create a comment because I cannot enter the "#" character.

>> When I try to type "#", the status bar simply shows "Unknown Command".

>>

>> I don't really understand the change, but if in the

>> MainEditorKeyInput function in file MainTextEditor.c I delete the "NoShiftState" check from the first "else if"

>> below:

>>

>> +        //

>> +        // dispatch to different components' key handling function

>> +        //

>> +        if (EFI_NOT_FOUND != MenuBarDispatchControlHotKey(&KeyData)) {

>> +          Status = EFI_SUCCESS;

>> +        } else if (NoShiftState && ((KeyData.Key.ScanCode ==

>> + SCAN_NULL) ||

>> ((KeyData.Key.ScanCode >= SCAN_UP) && (KeyData.Key.ScanCode <=

>> SCAN_PAGE_DOWN)))) {

>> +          Status = FileBufferHandleInput (&KeyData.Key);

>> +        } else if (NoShiftState && (KeyData.Key.ScanCode >= SCAN_F1)

>> + &&

>> (KeyData.Key.ScanCode <= SCAN_F12)) {

>> +          Status = MenuBarDispatchFunctionKey (&KeyData.Key);

>> +        } else {

>> +          StatusBarSetStatusString (L"Unknown Command");

>> +          FileBufferMouseNeedRefresh = FALSE;

>> +        }

>>

>> then I am able to enter "#" and other characters that I previously

>> was unable to enter.

>>

>> Can you have a look at this?  I assume any shell binary built with

>> this change will have a similar issue.

>>

>> Thanks,

>> Jim

>>

>>> -----Original Message-----

>>> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf

>>> Of Ruiyu Ni

>>> Sent: Monday, February 12, 2018 9:34 AM

>>> To: edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>

>>> Cc: Jaben Carsey; Felix

>>> Subject: [edk2] [PATCH] ShellPkg/[hex]edit: use SimpleTextInEx to

>>> read console

>>>

>>> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=682

>>>

>>> Edit and HexEdit commands assume that SimpleTxtIn translates

>>> Ctrl+<Alpha-Key> key combinations into Unicode control characters

>>> (0x1-0x1A).

>>>

>>> Such translation does not seem to be required by the UEFI spec.

>>> Shell should not rely on implementation specific behavior.

>>> It should instead use SimpleTextInEx to read Ctrl+<Alpha-Key> key combinations.

>>>

>>> The patch changes edit and hexedit to only consumes SimpleTextInEx.

>>>

>>> Contributed-under: TianoCore Contribution Agreement 1.1

>>> Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com<mailto:ruiyu.ni@intel.com>>

>>> Reported-by: Felix <felixp@mail.ru<mailto:felixp@mail.ru>>

>>> Cc: Felix <felixp@mail.ru<mailto:felixp@mail.ru>>

>>> Cc: Jaben Carsey <jaben.carsey@intel.com<mailto:jaben.carsey@intel.com>>

>>> ---

>>>  .../Edit/MainTextEditor.c                          | 135 +++++++++-----

>>>  .../Edit/TextEditorTypes.h                         |  21 ++-

>>>  .../UefiShellDebug1CommandsLib/EditInputBar.c      |  34 +++-

>>>  .../UefiShellDebug1CommandsLib/EditInputBar.h      |   6 +-

>>>  .../UefiShellDebug1CommandsLib/EditMenuBar.c       |  38 +++-

>>>  .../UefiShellDebug1CommandsLib/EditMenuBar.h       |   6 +-

>>>  .../HexEdit/HexEditorTypes.h                       |  25 +--

>>>  .../HexEdit/MainHexEditor.c                        | 205 +++++++++++++--------

>>>  8 files changed, 309 insertions(+), 161 deletions(-)

>>>

>>> ---------8<----- clip ----->8--------

>>>

>>> --

>>> 2.16.1.windows.1

>>>

>>> _______________________________________________

>>> edk2-devel mailing list

>>> edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>

>>> https://lists.01.org/mailman/listinfo/edk2-devel



_______________________________________________

edk2-devel mailing list

edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>

https://lists.01.org/mailman/listinfo/edk2-devel
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: How to Interpret ReadKeyStrokeEX Data
  2018-06-06 13:03     ` Jim.Dailey
@ 2018-06-06 13:30       ` Andrew Fish
  0 siblings, 0 replies; 12+ messages in thread
From: Andrew Fish @ 2018-06-06 13:30 UTC (permalink / raw)
  To: Jim.Dailey; +Cc: Michael Rothmam, Carsey, Jaben, ruiyu.ni, edk2-devel, felixp


> On Jun 6, 2018, at 6:03 AM, Jim.Dailey@dell.com wrote:
> 
> Thanks, Mike.  I figured that out yesterday.
> 
> I think the only problem now is that any shell built with the original change to edit will not work as expected if it is run on a system whose BIOS was created before the second change was made (i.e. pretty much every existing UEFI BIOS in the world!).
> 
> Since the shell is a stand-alone tool that is not necessarily tied to any particular machine, I think that it is likely that more than a few people may run into this problem in the future.
> 

Jim,

You bring up a really good issue about the compatibility targets for the EFI Shell.I'll bring this up at the next Tianocore Stewards meeting so we can put some more thought into what the right thing to do is. 

Thanks,

Andrew Fish


> Regards,
> Jim
> 
> -----Original Message-----
> From: Rothman, Michael A [mailto:michael.a.rothman@intel.com] 
> Sent: Tuesday, June 5, 2018 10:38 PM
> To: Rothman, Michael A; Dailey, Jim; Ni, Ruiyu
> Cc: Carsey, Jaben; edk2-devel@lists.01.org; felixp@mail.ru
> Subject: RE: How to Interpret ReadKeyStrokeEX Data
> 
> Jim,
> 
> I think the problem you're seeing is that the USB keyboard driver you're using is downrev and needs to be updated.
> 
> If you look at https://github.com/tianocore/edk2/commit/dd190645eb43424706eb1709d0032c69a1935d9f there was a fix checked in to address exactly the issue you're running into. It's basically a symptom of running a new shell without a correspondingly updated keyboard driver. The new shell in effect exposed a latent bug.
> 
> Hope that helps.
> 
> Thanks,
> Mike Rothman 
> (迈克 罗斯曼 / माइकल रोथ्मेन् / Михаил Ротман / משה רוטמן)
> רועה עיקרי של חתולים
> 
> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Rothman, Michael A
> Sent: Monday, June 4, 2018 10:31 AM
> To: Jim.Dailey@dell.com; Ni, Ruiyu <ruiyu.ni@intel.com>
> Cc: Carsey, Jaben <jaben.carsey@intel.com>; edk2-devel@lists.01.org; felixp@mail.ru
> Subject: Re: [edk2] How to Interpret ReadKeyStrokeEX Data
> 
> Since I'm largely the person who might be to blame for the language and intent here and I’ll focus on the spec-related item.  See my comments below.
> 
> 
> 
> Thanks,
> 
> Mike Rothman
> 
> (迈克 罗斯曼 / माइकल रोथ्मेन् / Михаил Ротман / משה רוטמן)
> 
> רועה עיקרי של חתולים
> 
> 
> 
> 
> 
> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Jim.Dailey@dell.com
> Sent: Friday, June 1, 2018 11:27 AM
> To: Ni, Ruiyu <ruiyu.ni@intel.com>
> Cc: Carsey, Jaben <jaben.carsey@intel.com>; edk2-devel@lists.01.org; felixp@mail.ru
> Subject: [edk2] How to Interpret ReadKeyStrokeEX Data
> 
> 
> 
> (Subject changed)
> 
> 
> 
> I guess this is a question of UEFI spec interpretation.  In the Console I/O Protocol description of Version 2.5 of the spec (page 456), it says:
> 
> 
> 
>    If the Scan Code is set to 0x00 then the Unicode character is
> 
>    valid and should be used.
> 
> 
> 
> To me that clearly says it all.  The shift modifier is a don't care when the scan code is zero.  And, this change in the shell code seems to be a violation of that statement.
> 
> 
> 
> However, I also see some confusing (and grammatically incorrect) text in the description of the ReadKeyStrokeEx() function of the simple text in protocol that I am guessing is related to this change (*emphasis* mine):
> 
> 
> 
>    When interpreting the data from this function, it should be
> 
>    noted that if a class of printable characters that are normally
> 
>    *adjusted* by shift modifiers (e.g. Shift Key + "f" key) would
> 
>    be presented solely as a KeyData.Key.UnicodeChar without the
> 
>    associated shift state.
> 
> 
> 
> What I think the spec is trying to say here is that for A-Z and a-z, the consumer does NOT need to look at the shift state to tell "A" from "a", for example, because the Unicode character will be either "A" or "a" as appropriate.
> 
> 
> 
> n  No, it is any key that would create a displayable character that adjusts based on the shift state. Realize that the users of ReadKeyStroke/ReadKeyStrokeEx fall back to a common denominator of Scan Code or Unicode Character. So if someone types a shift 4, the underlying keyboard layout that the keyboard driver controls would dictate how that gets translated. On my keyboard in the US it turns into a “$” symbol, while someone in Europe may very well have a software-defined keyboard layout which translates the same keystroke to a “€” symbol. That of course applies to the many characters you specified (A-F, a-f) and many others.
> 
> n  The text above is intended to imply what it says, “a class of printable characters … would be presented solely as a KeyData.Key.UnicodeChar without the associated shift state. This makes consumers of both the Ex and Non-Ex variant of ReadKeyStroke able to use the same logic to test for any shiftable characters by simply looking at the Unicode value. I’d note the shift and toggle states (which are only available on Ex) are there not so much for interpreting the translated key but to maximize flexibility associated with keyboard mapping as a UEFI application.
> 
> 
> 
> I think saying this is unnecessary simply because the earlier statement (If the Scan Code is set to 0x00 then the Unicode character is valid and should be used.) covers this case.
> 
> 
> 
> Further, I believe this text applies only to the A-Z keys because their corresponding characters are *adjusted* (to upper case) when the shift key is pressed. That is, if you adjust "blue" to "BLUE", you have two different appearances, but only one meaning.
> 
> 
> 
> However, a "3" is not *adjusted* to a "#"; they are totally different characters with different meanings altogether. No C pre-processor would be happy seeing: "3ifdef SYMBOL".
> 
> 
> 
> In any case, I see nothing gained by ignoring keys having a zero scan code and a valid Unicode character; the spec says that "the Unicode character is valid and should be used".
> 
> 
> 
> Regards,
> 
> Jim
> 
> 
> 
>> -----Original Message-----
> 
>> From: Ni, Ruiyu [mailto:ruiyu.ni@intel.com]
> 
>> Sent: Thursday, May 31, 2018 7:19 PM
> 
>> To: Dailey, Jim
> 
>> Cc: Carsey, Jaben; felixp@mail.ru<mailto:felixp@mail.ru>; edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
> 
>> Subject: RE: [edk2] [PATCH] ShellPkg/[hex]edit: use SimpleTextInEx to
> 
>> read console
> 
>> 
> 
>> Can you check which keyboard driver are you using?
> 
>> The keyboard driver is expected to translate "SHIFT" + "3" to "#" (without Shift state).
> 
>> I know that some keyboard driver doesn't do that correctly.
> 
>> E.g.: SHIFT + "3" is translated to "#" but the SHIFT state is not masked off.
> 
>> 
> 
>> [Sorry I thought I sent the mail out days ago]
> 
>> 
> 
>>> -----Original Message-----
> 
>>> From: Jim.Dailey@dell.com<mailto:Jim.Dailey@dell.com> [mailto:Jim.Dailey@dell.com]
> 
>>> Sent: Wednesday, May 23, 2018 3:01 AM
> 
>>> To: Ni, Ruiyu <ruiyu.ni@intel.com<mailto:ruiyu.ni@intel.com>>
> 
>>> Cc: Carsey, Jaben <jaben.carsey@intel.com<mailto:jaben.carsey@intel.com>>; felixp@mail.ru<mailto:felixp@mail.ru>; edk2-
> 
>>> devel@lists.01.org<mailto:devel@lists.01.org>
> 
>>> Subject: RE: [edk2] [PATCH] ShellPkg/[hex]edit: use SimpleTextInEx to
> 
>>> read console
> 
>>> 
> 
>>> Ray,
> 
>>> 
> 
>>> The patch in the message below was applied to the tree on 12 Feb this
> 
>>> year (5563281fa2b31093a1cbd415553b9264c5136e89).
> 
>>> 
> 
>>> Part of the change to MainTextEditor.c causes an issue where I cannot
> 
>>> enter (at least some) shifted punctuation.  For example, after this
> 
>>> check in I cannot edit a shell script and create a comment because I cannot enter the "#" character.
> 
>>> When I try to type "#", the status bar simply shows "Unknown Command".
> 
>>> 
> 
>>> I don't really understand the change, but if in the
> 
>>> MainEditorKeyInput function in file MainTextEditor.c I delete the "NoShiftState" check from the first "else if"
> 
>>> below:
> 
>>> 
> 
>>> +        //
> 
>>> +        // dispatch to different components' key handling function
> 
>>> +        //
> 
>>> +        if (EFI_NOT_FOUND != MenuBarDispatchControlHotKey(&KeyData)) {
> 
>>> +          Status = EFI_SUCCESS;
> 
>>> +        } else if (NoShiftState && ((KeyData.Key.ScanCode ==
> 
>>> + SCAN_NULL) ||
> 
>>> ((KeyData.Key.ScanCode >= SCAN_UP) && (KeyData.Key.ScanCode <=
> 
>>> SCAN_PAGE_DOWN)))) {
> 
>>> +          Status = FileBufferHandleInput (&KeyData.Key);
> 
>>> +        } else if (NoShiftState && (KeyData.Key.ScanCode >= SCAN_F1)
> 
>>> + &&
> 
>>> (KeyData.Key.ScanCode <= SCAN_F12)) {
> 
>>> +          Status = MenuBarDispatchFunctionKey (&KeyData.Key);
> 
>>> +        } else {
> 
>>> +          StatusBarSetStatusString (L"Unknown Command");
> 
>>> +          FileBufferMouseNeedRefresh = FALSE;
> 
>>> +        }
> 
>>> 
> 
>>> then I am able to enter "#" and other characters that I previously
> 
>>> was unable to enter.
> 
>>> 
> 
>>> Can you have a look at this?  I assume any shell binary built with
> 
>>> this change will have a similar issue.
> 
>>> 
> 
>>> Thanks,
> 
>>> Jim
> 
>>> 
> 
>>>> -----Original Message-----
> 
>>>> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf
> 
>>>> Of Ruiyu Ni
> 
>>>> Sent: Monday, February 12, 2018 9:34 AM
> 
>>>> To: edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
> 
>>>> Cc: Jaben Carsey; Felix
> 
>>>> Subject: [edk2] [PATCH] ShellPkg/[hex]edit: use SimpleTextInEx to
> 
>>>> read console
> 
>>>> 
> 
>>>> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=682
> 
>>>> 
> 
>>>> Edit and HexEdit commands assume that SimpleTxtIn translates
> 
>>>> Ctrl+<Alpha-Key> key combinations into Unicode control characters
> 
>>>> (0x1-0x1A).
> 
>>>> 
> 
>>>> Such translation does not seem to be required by the UEFI spec.
> 
>>>> Shell should not rely on implementation specific behavior.
> 
>>>> It should instead use SimpleTextInEx to read Ctrl+<Alpha-Key> key combinations.
> 
>>>> 
> 
>>>> The patch changes edit and hexedit to only consumes SimpleTextInEx.
> 
>>>> 
> 
>>>> Contributed-under: TianoCore Contribution Agreement 1.1
> 
>>>> Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com<mailto:ruiyu.ni@intel.com>>
> 
>>>> Reported-by: Felix <felixp@mail.ru<mailto:felixp@mail.ru>>
> 
>>>> Cc: Felix <felixp@mail.ru<mailto:felixp@mail.ru>>
> 
>>>> Cc: Jaben Carsey <jaben.carsey@intel.com<mailto:jaben.carsey@intel.com>>
> 
>>>> ---
> 
>>>> .../Edit/MainTextEditor.c                          | 135 +++++++++-----
> 
>>>> .../Edit/TextEditorTypes.h                         |  21 ++-
> 
>>>> .../UefiShellDebug1CommandsLib/EditInputBar.c      |  34 +++-
> 
>>>> .../UefiShellDebug1CommandsLib/EditInputBar.h      |   6 +-
> 
>>>> .../UefiShellDebug1CommandsLib/EditMenuBar.c       |  38 +++-
> 
>>>> .../UefiShellDebug1CommandsLib/EditMenuBar.h       |   6 +-
> 
>>>> .../HexEdit/HexEditorTypes.h                       |  25 +--
> 
>>>> .../HexEdit/MainHexEditor.c                        | 205 +++++++++++++--------
> 
>>>> 8 files changed, 309 insertions(+), 161 deletions(-)
> 
>>>> 
> 
>>>> ---------8<----- clip ----->8--------
> 
>>>> 
> 
>>>> --
> 
>>>> 2.16.1.windows.1
> 
>>>> 
> 
>>>> _______________________________________________
> 
>>>> edk2-devel mailing list
> 
>>>> edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
> 
>>>> https://lists.01.org/mailman/listinfo/edk2-devel
> 
> 
> 
> _______________________________________________
> 
> edk2-devel mailing list
> 
> edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
> 
> https://lists.01.org/mailman/listinfo/edk2-devel
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel



^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2018-06-06 13:30 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-06-01 18:26 How to Interpret ReadKeyStrokeEX Data Jim.Dailey
2018-06-04  3:00 ` Ni, Ruiyu
2018-06-04 14:46   ` Jim.Dailey
2018-06-04 14:49     ` Andrew Fish
2018-06-04 16:00       ` Jim.Dailey
2018-06-04 16:02         ` Jim.Dailey
2018-06-04 16:21           ` Carsey, Jaben
2018-06-04 16:28             ` Jim.Dailey
2018-06-04 17:31 ` Rothman, Michael A
2018-06-06  3:37   ` Rothman, Michael A
2018-06-06 13:03     ` Jim.Dailey
2018-06-06 13:30       ` Andrew Fish

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox