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.roth...@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; fel...@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-boun...@lists.01.org] On Behalf Of Rothman, 
Michael A
Sent: Monday, June 4, 2018 10:31 AM
To: jim.dai...@dell.com; Ni, Ruiyu <ruiyu...@intel.com>
Cc: Carsey, Jaben <jaben.car...@intel.com>; edk2-devel@lists.01.org; 
fel...@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-boun...@lists.01.org] On Behalf Of 
jim.dai...@dell.com
Sent: Friday, June 1, 2018 11:27 AM
To: Ni, Ruiyu <ruiyu...@intel.com>
Cc: Carsey, Jaben <jaben.car...@intel.com>; edk2-devel@lists.01.org; 
fel...@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...@intel.com]

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

> To: Dailey, Jim

> Cc: Carsey, Jaben; fel...@mail.ru<mailto:fel...@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.dai...@dell.com<mailto:jim.dai...@dell.com> 
>> [mailto:jim.dai...@dell.com]

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

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

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

>> de...@lists.01.org<mailto:de...@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-boun...@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...@intel.com<mailto:ruiyu...@intel.com>>

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

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

>>> Cc: Jaben Carsey <jaben.car...@intel.com<mailto:jaben.car...@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

Reply via email to