Jaben, I've attached an updated patch and .uni file for review and submit (same commit text as I originally sent below). I made your suggested change below, and updated the help text.
If Y and N are not used everywhere, then I think leaving the non-SFO characters alone is preferred. Thanks, Chris From: Carsey, Jaben [mailto:jaben.car...@intel.com] Sent: Friday, August 22, 2014 11:21 AM To: Phillips, Chris J (Plano, TX); edk2-devel@lists.sourceforge.net Cc: Carsey, Jaben Subject: RE: ShellPkg: Fixes and updates for 'devices' command Chris, I agree that it's not defined. It's really the N's that I think make reading it quite hard. You could leave the Y. Really I like the whitespace instead of the N to make finding the other characters easier. -Jaben From: Phillips, Chris J (Plano, TX) [mailto:chr...@hp.com] Sent: Friday, August 22, 2014 9:07 AM To: Carsey, Jaben; edk2-devel@lists.sourceforge.net<mailto:edk2-devel@lists.sourceforge.net> Subject: RE: ShellPkg: Fixes and updates for 'devices' command Importance: High The - and X are not defined in the spec. Anyone new to the shell cannot tell the difference between what these mean. Someone could easily think that X means the protocol is not supported by the driver. I didn't think it would be controversial to have the non-SFO match SFO, and use the clearly specified Y and N characters. I'm interested to hear if anyone else has an opinion on this. In the meantime, I'll update the patch to revert this change, and update the help output to state the meaning of - and X. Thanks, Chris From: Carsey, Jaben [mailto:jaben.car...@intel.com] Sent: Friday, August 22, 2014 10:43 AM To: Phillips, Chris J (Plano, TX); edk2-devel@lists.sourceforge.net<mailto:edk2-devel@lists.sourceforge.net> Cc: Carsey, Jaben Subject: RE: ShellPkg: Fixes and updates for 'devices' command I really disagree about readability. I find the Y/N in the non-SFO table significantly harder to find the Y in a field of N. I think this makes it very hard to use the command. We have lots of users who use this command today and I do not understand why we are changing something that exists and works already just to make it the same as the machine readable output which is designed to be parsed by a command. -Jaben From: Phillips, Chris J (Plano, TX) [mailto:chr...@hp.com] Sent: Thursday, August 21, 2014 8:12 PM To: Carsey, Jaben; edk2-devel@lists.sourceforge.net<mailto:edk2-devel@lists.sourceforge.net> Subject: RE: ShellPkg: Fixes and updates for 'devices' command Importance: High Jaben, This change was made on purpose so SFO and non-SFO use the same characters. When I was creating ECRs last year Mike said the example output for shell commands in the spec are just examples. Implementations can decide what and how to display. The SFO output is specified and for the 'devices' command Y and N are defined in the SFO table. Using Y and N in the non-SFO output is not a spec violation and allows users/scripts to look for the same characters. I would prefer to proceed with the update, for consistency between SFO and non-SFO output. Thanks, Chris From: Carsey, Jaben [mailto:jaben.car...@intel.com] Sent: Thursday, August 21, 2014 3:12 PM To: Phillips, Chris J (Plano, TX); edk2-devel@lists.sourceforge.net<mailto:edk2-devel@lists.sourceforge.net> Cc: Carsey, Jaben Subject: RE: ShellPkg: Fixes and updates for 'devices' command Chris, The SFO output change actually accidentally messed up the non-SFO output per the spec. In Non-SFO mode, the Y/N are not used and X and - are used for present and non-present. The N and Y characters in the table are much bigger and at a glance the - and X is easier to tell apart. Could this get updated from : Cfg?L'Y':L'N', Diag?L'Y':L'N', To: Cfg?(SfoFlag?L'Y':L'X'):(SfoFlag?L'N':L'-'), Diag?(SfoFlag?L'Y':L'X'):( SfoFlag?L'N':L'-'), From: Phillips, Chris J (Plano, TX) [mailto:chr...@hp.com] Sent: Wednesday, August 20, 2014 11:44 AM To: Carsey, Jaben; edk2-devel@lists.sourceforge.net<mailto:edk2-devel@lists.sourceforge.net> Subject: ShellPkg: Fixes and updates for 'devices' command Importance: High Please review the attached patch. I'm also including the .uni file. Thanks, Chris ShellPkg: Fixes and updates for the 'devices' command - Update 'devices -sfo' format to match UEFI Shell 2.1 spec - Update help output for easier viewing - Update 'devices' output format for better alignment when there are many device handles: T D Y C I P F A CTRL E G G #P #D #C Device Name ==== = = = == == === ========================================================= Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Chris Phillips <chr...@hp.com<mailto:chr...@hp.com>>
DevicesFixes3.patch
Description: DevicesFixes3.patch
UefiShellDriver1CommandsLib.uni
Description: UefiShellDriver1CommandsLib.uni
------------------------------------------------------------------------------ Slashdot TV. Video for Nerds. Stuff that matters. http://tv.slashdot.org/
_______________________________________________ edk2-devel mailing list edk2-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/edk2-devel
