[PATCH 1/1] cli: always show cursor

[Heinrich Schuchardt]

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.

Best regards

Heinrich

> 
> Regards,
> SImon