Re: [PATCH] MdeModulePkg/UefiBootManagerLib: Limit the boot description to 72 chars

["Gao, Zhichao" <zhichao.gao@intel.com>]

Well. That's another solution to limit the boot option showing in one line. I can implement this.

Thanks,
Zhichao

> -----Original Message-----
> From: Ni, Ray <ray.ni@intel.com>
> Sent: Thursday, September 9, 2021 1:02 AM
> To: Gao, Zhichao <zhichao.gao@intel.com>; devel@edk2.groups.io
> Cc: Wang, Jian J <jian.j.wang@intel.com>; Liming Gao
> <gaoliming@byosoft.com.cn>
> Subject: RE: [PATCH] MdeModulePkg/UefiBootManagerLib: Limit the boot
> description to 72 chars
> 
> Zhichao,
> I didn't mean to update UI logic to support scrolling. I meant to update BM
> logic to show partial string of boot description.
> 
> Thanks,
> Ray
> 
> > -----Original Message-----
> > From: Gao, Zhichao <zhichao.gao@intel.com>
> > Sent: Wednesday, September 8, 2021 10:29 AM
> > To: Ni, Ray <ray.ni@intel.com>; devel@edk2.groups.io
> > Cc: Wang, Jian J <jian.j.wang@intel.com>; Liming Gao
> > <gaoliming@byosoft.com.cn>
> > Subject: RE: [PATCH] MdeModulePkg/UefiBootManagerLib: Limit the boot
> > description to 72 chars
> >
> > Hi Ray,
> >
> > Agree with you. I used to think of change the display logic of the BM
> > app. But it is not tiny. It need to update the scroll bar definition and one
> more trace for the item string lines. I would prefer to defer it.
> > The limitation of the boot description length is required. I don't
> > know why some USB disk would have a very long serial number string. On
> > my opinion, 72 characters is enough for the boot option. If there is no
> limitation, the boot description can be long enough that one screen can not
> even show one item.
> > 80 * 25 is the minimum mode for most terminal console device. That is why
> I choose it and add the limitation.
> >
> > Thanks,
> > Zhichao
> >
> > > -----Original Message-----
> > > From: Ni, Ray <ray.ni@intel.com>
> > > Sent: Tuesday, September 7, 2021 3:54 PM
> > > To: Gao, Zhichao <zhichao.gao@intel.com>; devel@edk2.groups.io
> > > Cc: Wang, Jian J <jian.j.wang@intel.com>; Liming Gao
> > > <gaoliming@byosoft.com.cn>
> > > Subject: RE: [PATCH] MdeModulePkg/UefiBootManagerLib: Limit the
> boot
> > > description to 72 chars
> > >
> > > Zhichao,
> > > The mode 80x25 is platform specific. I don't prefer to update common
> > > logic to have such limitation.
> > > Can you update the UI logic of displaying the boot option list?
> > >
> > > > -----Original Message-----
> > > > From: Gao, Zhichao <zhichao.gao@intel.com>
> > > > Sent: Tuesday, September 7, 2021 2:13 PM
> > > > To: devel@edk2.groups.io
> > > > Cc: Wang, Jian J <jian.j.wang@intel.com>; Liming Gao
> > > > <gaoliming@byosoft.com.cn>; Ni, Ray <ray.ni@intel.com>
> > > > Subject: [PATCH] MdeModulePkg/UefiBootManagerLib: Limit the boot
> > > > description to 72 chars
> > > >
> > > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3590
> > > >
> > > > The minimum mode is 80*25 which means echo line can have 80
> > > > charactors and max 25 line on the screen. And the BootManagerMeu
> > > > see each boot option as one line. The BootManagerMenuApp would
> > > > have 2 charactors for draw box and 6 charactors for space. So it
> > > > is better to limit the boot description within 72 charactors.
> > > >
> > > > Cc: Jian J Wang <jian.j.wang@intel.com>
> > > > Cc: Liming Gao <gaoliming@byosoft.com.cn>
> > > > Cc: Ray Ni <ray.ni@intel.com>
> > > > Signed-off-by: Zhichao Gao <zhichao.gao@intel.com>
> > > > ---
> > > >  .../Library/UefiBootManagerLib/BmBootDescription.c | 24
> > > > ++++++++++++++++++----
> > > >  1 file changed, 20 insertions(+), 4 deletions(-)
> > > >
> > > > diff --git
> > > > a/MdeModulePkg/Library/UefiBootManagerLib/BmBootDescription.c
> > > > b/MdeModulePkg/Library/UefiBootManagerLib/BmBootDescription.c
> > > > index aa891feb17..7260b2a203 100644
> > > > ---
> a/MdeModulePkg/Library/UefiBootManagerLib/BmBootDescription.c
> > > > +++
> b/MdeModulePkg/Library/UefiBootManagerLib/BmBootDescription.c
> > > > @@ -1,7 +1,7 @@
> > > >  /** @file
> > > >
> > > >    Library functions which relate with boot option description.
> > > >
> > > >
> > > >
> > > > -Copyright (c) 2011 - 2018, Intel Corporation. All rights
> > > > reserved.<BR>
> > > >
> > > > +Copyright (c) 2011 - 2021, Intel Corporation. All rights
> > > > +reserved.<BR>
> > > >
> > > >  (C) Copyright 2015 Hewlett Packard Enterprise Development LP<BR>
> > > >
> > > >  SPDX-License-Identifier: BSD-2-Clause-Patent
> > > >
> > > >
> > > >
> > > > @@ -14,6 +14,8 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
> > > >  #define PRODUCT_IDENTIFICATION_OFFSET    11
> > > >
> > > >  #define PRODUCT_IDENTIFICATION_LENGTH    16
> > > >
> > > >
> > > >
> > > > +#define MAX_BOOT_DESCRIPTION_LEGNTH      72
> > > >
> > > > +
> > > >
> > > >  CONST UINT16 mBmUsbLangId    = 0x0409; // English
> > > >
> > > >  CHAR16       mBmUefiPrefix[] = L"UEFI ";
> > > >
> > > >
> > > >
> > > > @@ -773,6 +775,7 @@ BmGetBootDescription (
> > > >    CHAR16                         *DefaultDescription;
> > > >
> > > >    CHAR16                         *Temp;
> > > >
> > > >    UINTN                          Index;
> > > >
> > > > +  UINTN                          DescriptionLen;
> > > >
> > > >
> > > >
> > > >    //
> > > >
> > > >    // Firstly get the default boot description
> > > >
> > > > @@ -785,10 +788,23 @@ BmGetBootDescription (
> > > >        // Avoid description confusion between UEFI & Legacy boot
> > > > option by adding "UEFI " prefix
> > > >
> > > >        // ONLY for core provided boot description handler.
> > > >
> > > >        //
> > > >
> > > > -      Temp = AllocatePool (StrSize (DefaultDescription) + sizeof
> > > (mBmUefiPrefix));
> > > >
> > > > +      if (StrLen (DefaultDescription) + StrLen (mBmUefiPrefix) >
> > > > + MAX_BOOT_DESCRIPTION_LEGNTH) {
> > > >
> > > > +        //
> > > >
> > > > +        // Limit the MAX length of boot description to 72 charactors.
> > > >
> > > > +        // Replace the last 3 charactors to L"...".
> > > >
> > > > +        //
> > > >
> > > > +        DescriptionLen = MAX_BOOT_DESCRIPTION_LEGNTH + 1;
> > > >
> > > > +        DefaultDescription[MAX_BOOT_DESCRIPTION_LEGNTH - 5 - 3] =
> > > > + L'.';
> > > >
> > > > +        DefaultDescription[MAX_BOOT_DESCRIPTION_LEGNTH - 5 - 2] =
> > > > + L'.';
> > > >
> > > > +        DefaultDescription[MAX_BOOT_DESCRIPTION_LEGNTH - 5 - 1] =
> > > > + L'.';
> > > >
> > > > +        DefaultDescription[MAX_BOOT_DESCRIPTION_LEGNTH - 5] =
> > > > + L'\0';
> > > >
> > > > +      } else {
> > > >
> > > > +        DescriptionLen = (StrSize (DefaultDescription) + sizeof
> > > > + (mBmUefiPrefix)) / sizeof (CHAR16);
> > > >
> > > > +      }
> > > >
> > > > +      Temp = AllocatePool (DescriptionLen * sizeof (CHAR16));
> > > >
> > > >        ASSERT (Temp != NULL);
> > > >
> > > > -      StrCpyS (Temp, (StrSize (DefaultDescription) + sizeof
> (mBmUefiPrefix))
> > > / sizeof (CHAR16), mBmUefiPrefix);
> > > >
> > > > -      StrCatS (Temp, (StrSize (DefaultDescription) + sizeof
> (mBmUefiPrefix))
> > > / sizeof (CHAR16), DefaultDescription);
> > > >
> > > > +      StrCpyS (Temp, DescriptionLen, mBmUefiPrefix);
> > > >
> > > > +      StrCatS (Temp, DescriptionLen, DefaultDescription);
> > > >
> > > >        FreePool (DefaultDescription);
> > > >
> > > >        DefaultDescription = Temp;
> > > >
> > > >        break;
> > > >
> > > > --
> > > > 2.16.2.windows.1