[PATCH 1/1] cli: always show cursor

[Simon Glass]

Hi Heinrich,

On Sat, 29 Oct 2022 at 20:56, Heinrich Schuchardt
<heinrich.schuchardt at canonical.com> wrote:
>
>
>
> On 10/30/22 02:43, Simon Glass wrote:
> > 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)
>
> A program outputting different things in continuous integration then in
> real live does not provide realistic testing.
>
> It is the CI that has to adapt not the program under test.
>
> Which logs are you talking about?
>
> > 2. We cannot show an ANSI code on boot by default, since that puts
> > ctrl characters into every start-up of U-Boot
>
> Why would that be a problem?
>
> >
> > 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.
>
> isatty() is not a U-Boot function. So it is irrelevant in this context.
>
> You cannot detect if a serial console understands ANSI sequences without
> sending any.

NAK

Please try a little harder to understand my POV above.

Regards,
Simon