Re: [PATCH 0/3] MdeModulePkg/TerminalDxe: TtyTerm improvements

["Tian, Feng" <feng.tian@intel.com>]

Sorry for missing this patch.

Reviewed-by: Feng Tian <feng.tian@intel.com>

I will help push it in.

Thanks
Feng

-----Original Message-----
From: Brian J. Johnson [mailto:bjohnson@sgi.com] 
Sent: Wednesday, October 26, 2016 11:00 PM
To: Tian, Feng <feng.tian@intel.com>; Zeng, Star <star.zeng@intel.com>
Cc: Roy Franz <roy.franz@hpe.com>; Ryan Harkin <ryan.harkin@linaro.org>; Laszlo Ersek <lersek@redhat.com>; edk2-devel@lists.01.org <edk2-devel@ml01.01.org>; Leif Lindholm <leif.lindholm@linaro.org>
Subject: Re: [edk2] [PATCH 0/3] MdeModulePkg/TerminalDxe: TtyTerm improvements

On 10/18/2016 10:34 AM, Brian J. Johnson wrote:
> On 10/14/2016 03:37 PM, Laszlo Ersek wrote:
>> On 10/14/16 21:39, Brian J. Johnson wrote:
>>> On 10/12/2016 03:17 AM, Ryan Harkin wrote:
>>>> On 7 October 2016 at 16:59, Leif Lindholm 
>>>> <leif.lindholm@linaro.org>
>>>> wrote:
>>>>> Roy can now be found at Roy Franz <roy.franz@hpe.com> (cc:d).
>>>>>
>>>>> On Fri, Oct 07, 2016 at 05:56:26PM +0200, Laszlo Ersek wrote:
>>>>>> Roy, Ryan,
>>>>>>
>>>>>> On 10/07/16 16:53, Brian J. Johnson wrote:
>>>>>>> This patch series implements some improvements to the TtyTerm 
>>>>>>> terminal type in the TerminalDxe driver.  It fixes an end case 
>>>>>>> with cursor position tracking, and uses that to optimize cursor 
>>>>>>> motion escape sequences.  It also adds support for the page up, 
>>>>>>> page down, insert, home, and end keys on some additional common 
>>>>>>> terminal emulators.
>>>>>>>
>>>>>>> The result is improved performance, especially at the shell 
>>>>>>> prompt, and better compatibility with common terminal emulators.  
>>>>>>> In particular, as a side effect of the optimized cursor motion, 
>>>>>>> terminal windows which are taller than the current mode setting 
>>>>>>> (eg. 25
>>>>>>> lines)
>>>>>>> work much better than before.
>>>>>>>
>>>>>>> Most of these fixes have been in production in some form on 
>>>>>>> SGI's servers for years.
>>>>>>>
>>>>>>> Contributed-under: TianoCore Contribution Agreement 1.0
>>>>>>> Signed-off-by: Brian Johnson <bjohnson@sgi.com>
>>>>>>> Cc: Feng Tian <feng.tian@intel.com>
>>>>>>> Cc: Star Zeng <star.zeng@intel.com>
>>>>>>>
>>>>>>> Brian J. Johnson (3):
>>>>>>>   MdeModulePkg/TerminalDxe: Improve TtyTerm cursor position tracking
>>>>>>>   MdeModulePkg/TerminalDxe: Optimize TtyTerm cursor motion
>>>>>>>   MdeModulePkg/TerminalDxe: Handle more keys with TtyTerm
>>>>>>>
>>>>>>>  .../Universal/Console/TerminalDxe/Terminal.h       |  2 +
>>>>>>>  .../Universal/Console/TerminalDxe/TerminalConIn.c  | 24 
>>>>>>> +++++++--  .../Universal/Console/TerminalDxe/TerminalConOut.c | 
>>>>>>> 61
>>>>>>> ++++++++++++++++++++--
>>>>>>>  3 files changed, 79 insertions(+), 8 deletions(-)
>>>>>>>
>>>>>>
>>>>>> can you please provide feedback (testing or otherwise) on this 
>>>>>> series?
>>>>>>
>>>>
>>>> Well, they "work" for me and I'd be happy with them being submitted.
>>>>
>>>> Tested-by: Ryan Harkin <ryan.harkin@linaro.org>
>>>>
>>>> The only curious effect I can see is the Print(L"xxx"); lines that 
>>>> expect the \n to be missing will no longer "work".  For example, I 
>>>> carry a patch by Daniil Egranov titled
>>>> "IntelFrameworkModulePkg/BdsDxe: Show boot timeout message" and it 
>>>> no longer displays the countdown on the same line each time, it 
>>>> prints each message on a new line.
>>>>
>>>> However, I don't see that as a blocking point, Daniil's patch could 
>>>> be changed easily and there are other advantages to this series 
>>>> that make it worthwhile, IMO, eg, Shell commands with lots of 
>>>> output (like "help" or "dir fs0:") no longer create an awful mess 
>>>> on the serial console.
>>>>
>>>
>>> So, is this just waiting for a maintainer's review?
>>
>> That's my understanding, yes.
>
> Feng or Star, could you please review these changes?

Ping?
-- 

						Brian J. Johnson

--------------------------------------------------------------------

   My statements are my own, are not authorized by SGI, and do not
   necessarily represent SGI’s positions.