[pacman-dev] [PATCH v2] Hide cursor while pacman is running

[Anatol Pomozov]

Hello

On Sat, Mar 7, 2020 at 9:55 PM Allan McRae <allan at archlinux.org> wrote:
>
> On 6/3/20 10:39 am, Anatol Pomozov wrote:
> > Use ASCII control codes to hide cursor at the pacman start and then
> > show the cursor when pacman finishes.
> >
> > It helps to avoid annoying blinking when progress bars are re-drawn.
> >
> > Cursor is reenabled if pacman expects user's input.
> >
> > Signed-off-by: Anatol Pomozov <anatol.pomozov at gmail.com>
> > ---
> >  src/common/util-common.c | 22 ++++++++++++++++++++++
> >  src/common/util-common.h |  3 +++
> >  src/pacman/pacman.c      |  2 ++
> >  src/pacman/sighandler.c  |  5 +++++
> >  4 files changed, 32 insertions(+)
> >
> > diff --git a/src/common/util-common.c b/src/common/util-common.c
> > index 7d43ac0d..09a41ea6 100644
> > --- a/src/common/util-common.c
> > +++ b/src/common/util-common.c
> > @@ -21,6 +21,7 @@
> >  #include <errno.h>
> >  #include <stdlib.h>
> >  #include <string.h>
> > +#include <unistd.h>
> >
> >  #include "util-common.h"
> >
> > @@ -104,6 +105,18 @@ int llstat(char *path, struct stat *buf)
> >       return ret;
> >  }
> >
> > +void console_hide_cursor(void) {
> > +     if(isatty(fileno(stdout))) {
>
> I don't think we should run this every line we read.

We actually need this particular check when we print the line.

The rationale here is to avoid printing the escape sequence if user
redirects pacman output to a file. Escape symbols make sense only for
a terminal.

> In fact, I doubt that is needed anywhere in libalpm currently.
> safe_fgets is used to read the localdb and when grepping install files
> for functions.  And libalpm should really not output anything...
>
> > +             printf("\x1B[?25l");
> > +     }
> > +}
> > +
> > +void console_show_cursor(void) {
> > +     if(isatty(fileno(stdout))) {
> > +             printf("\x1B[?25h");
> > +     }
> > +}
> > +
> >  /** Wrapper around fgets() which properly handles EINTR
> >   * @param s string to read into
> >   * @param size maximum length to read
> > @@ -114,6 +127,12 @@ char *safe_fgets(char *s, int size, FILE *stream)
> >  {
> >       char *ret;
> >       int errno_save = errno, ferror_save = ferror(stream);
> > +     int is_tty = isatty(fileno(stream));
> > +
> > +     if(is_tty) > +          /* Show cursor if read from STDIN */
> > +             console_show_cursor();
> > +     }
>
> I don't like having to do this every time we read something.  And now we
> are checking that "stream" is a tty and then "stdout" is a tty in the
> console_*_cursor() functions.
>
> It would be better to add a "safe_fgets_stdin()" in src/pacman/util.c
> that wraps safe_fgets() and does the cursor on/off when pacman is asking
> for input.  That is where we need it.

Yep, it makes sense. PTAL to upcoming patch.