[PATCH 1/1] cli: always show cursor

[Simon Glass]

Hi Heinrich,

On Thu, 27 Oct 2022 at 10:41, Heinrich Schuchardt
<heinrich.schuchardt at canonical.com> wrote:
>
>
>
> On 10/27/22 17:22, Simon Glass wrote:
> > Hi Heinrich,
> >
> > On Wed, 26 Oct 2022 at 00:13, Heinrich Schuchardt
> > <heinrich.schuchardt at canonical.com> wrote:
> >>
> >>
> >>
> >> On 10/26/22 01:35, Simon Glass wrote:
> >>> Hi Heinrich,
> >>>
> >>> On Sat, 22 Oct 2022 at 03:21, Heinrich Schuchardt
> >>> <heinrich.schuchardt at canonical.com> wrote:
> >>>>
> >>>> We may enter the command line interface in a state where on the remote
> >>>> console the cursor is not shown. Send an escape sequence to enable it.
> >>>>
> >>>> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt at canonical.com>
> >>>> ---
> >>>>    common/main.c | 4 ++++
> >>>>    1 file changed, 4 insertions(+)
> >>>>
> >>>> diff --git a/common/main.c b/common/main.c
> >>>> index 682f3359ea..4e7e7b17d6 100644
> >>>> --- a/common/main.c
> >>>> +++ b/common/main.c
> >>>> @@ -7,6 +7,7 @@
> >>>>    /* #define     DEBUG   */
> >>>>
> >>>>    #include <common.h>
> >>>> +#include <ansi.h>
> >>>>    #include <autoboot.h>
> >>>>    #include <bootstage.h>
> >>>>    #include <cli.h>
> >>>> @@ -66,6 +67,9 @@ void main_loop(void)
> >>>>
> >>>>           autoboot_command(s);
> >>>>
> >>>> +       if (!CONFIG_IS_ENABLED(DM_VIDEO) || CONFIG_IS_ENABLED(VIDEO_ANSI))
> >>>> +               printf(ANSI_CURSOR_SHOW "\n");
> >>>
> >>> Could we create a library which emits these codes? Like ansi_cursor_show() ?
> >>
> >> The question is not if we could but if we should.
> >>
> >> Up to now we tried to call printf() only once where ANSI_* is only one
> >> part of the output string.
> >>
> >> E.g. cmd/eficonfig.c:415:
> >>           printf(ANSI_CLEAR_CONSOLE
> >>                  ANSI_CURSOR_POSITION
> >>                  ANSI_CURSOR_SHOW, 1, 1);
> >>
> >> This minimizes the number of call library calls but may not be very elegant.
> >>
> >> Creating a library function for ANSI_CURSOR_SHOW alone does not make
> >> much sense to me. Should you want to convert our code base from using
> >> ANSI_* to library functions this should cover the whole code base.
> >>
> >> So I think we should let this patch pass as it solves a current problem
> >> and leave creating a ANSI_* function library to a separate exercise.
> >
> > Then we should add this to the serial API...what we have here is quite
> > bizarre in a way, since it outputs escape characters as the default
> > for U-Boot. Mostly we don't need it.
> >
> > So add a set_cursor_visible() method to the serial API and allow it to
> > be controlled via platform data in the serial driver.
>
> I have no clue which platform data you might be thinking of.
>
> It is also not my intention to eject the escape sequence whenever a
> serial console is probed.
>
> Using serial_puts() in this patch instead of printf() would help to
> avoid the CONFIG dependency.

Here's what should happen:

1. ANSI codes should not appear in logs or on CI (this is currently a
problem with EFI tests)
2. We cannot show an ANSI code on boot by default, since that puts
ctrl characters into every start-up of U-Boot

So I think my suggestion makes sense. See drivers/serial/sandbox.c and
have a way of calling isatty() to check the terminal type (with a
cmdline flag to override it, see -t for example). Then we can get the
correct behaviour.

Regards,
Simon