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

Message ID 20200308202253.40214-1-anatol.pomozov@gmail.com
State Superseded, archived
Headers show
Series
  • [pacman-dev,v2] Hide cursor while pacman is running
Related show

Commit Message

Anatol Pomozov March 8, 2020, 8:22 p.m. UTC
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@gmail.com>
---
 src/common/util-common.c | 13 +++++++++++++
 src/common/util-common.h |  3 +++
 src/pacman/pacman.c      |  2 ++
 src/pacman/sighandler.c  |  5 +++++
 src/pacman/util.c        | 19 ++++++++++++++-----
 src/pacman/util.h        |  1 +
 6 files changed, 38 insertions(+), 5 deletions(-)

Comments

Allan McRae March 9, 2020, 3:09 a.m. UTC | #1
On 9/3/20 6:22 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@gmail.com>
> ---
>  src/common/util-common.c | 13 +++++++++++++
>  src/common/util-common.h |  3 +++
>  src/pacman/pacman.c      |  2 ++
>  src/pacman/sighandler.c  |  5 +++++
>  src/pacman/util.c        | 19 ++++++++++++++-----
>  src/pacman/util.h        |  1 +
>  6 files changed, 38 insertions(+), 5 deletions(-)

I'm happy with the content of the patch.

Final request, move changes in src/common/util-common.* to
src/pacman/util.* - this should be entirely contained to pacman, not
libalpm.

Thanks,
Allan
Ralph Corderoy March 9, 2020, 9:48 a.m. UTC | #2
Hi,

> +void console_hide_cursor(void) {
> +       if(isatty(fileno(stdout))) {
> +               printf("\x1B[?25l");
> +       }

This is an ioctl(2) each time.
Can stdout be assumed not to change and just tested once?
Allan McRae March 9, 2020, 9:58 a.m. UTC | #3
On 9/3/20 7:48 pm, Ralph Corderoy wrote:
> Hi,
> 
>> +void console_hide_cursor(void) {
>> +       if(isatty(fileno(stdout))) {
>> +               printf("\x1B[?25l");
>> +       }
> 
> This is an ioctl(2) each time.
> Can stdout be assumed not to change and just tested once?

This is only called when pacman wants input.  Which is a small number of
times in a transaction, and the human is the rate limiting step.
Anatol Pomozov March 9, 2020, 6:59 p.m. UTC | #4
Hi

On Mon, Mar 9, 2020 at 2:58 AM Allan McRae <allan@archlinux.org> wrote:
>
> On 9/3/20 7:48 pm, Ralph Corderoy wrote:
> > Hi,
> >
> >> +void console_hide_cursor(void) {
> >> +       if(isatty(fileno(stdout))) {
> >> +               printf("\x1B[?25l");
> >> +       }
> >
> > This is an ioctl(2) each time.
> > Can stdout be assumed not to change and just tested once?
>
> This is only called when pacman wants input.  Which is a small number of
> times in a transaction, and the human is the rate limiting step.

Yep, this function is called at the pacman start and when we expect an
input from user i.e. a couple of times during `pacman` execution.

Allan, a new version of the patch has been sent to the maillist. PTAL.

Patch

diff --git a/src/common/util-common.c b/src/common/util-common.c
index 7d43ac0d..3850cffb 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))) {
+		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
diff --git a/src/common/util-common.h b/src/common/util-common.h
index 483d5da4..8eacde7e 100644
--- a/src/common/util-common.h
+++ b/src/common/util-common.h
@@ -28,6 +28,9 @@  char *mdirname(const char *path);
 
 int llstat(char *path, struct stat *buf);
 
+void console_hide_cursor(void);
+void console_show_cursor(void);
+
 char *safe_fgets(char *s, int size, FILE *stream);
 
 void wordsplit_free(char **ws);
diff --git a/src/pacman/pacman.c b/src/pacman/pacman.c
index d58c428a..aafd9e63 100644
--- a/src/pacman/pacman.c
+++ b/src/pacman/pacman.c
@@ -300,6 +300,7 @@  static void cleanup(int ret)
 
 	/* free memory */
 	FREELIST(pm_targets);
+	console_show_cursor();
 	exit(ret);
 }
 
@@ -1084,6 +1085,7 @@  int main(int argc, char *argv[])
 	int ret = 0;
 	uid_t myuid = getuid();
 
+	console_hide_cursor();
 	install_segv_handler();
 
 	/* i18n init */
diff --git a/src/pacman/sighandler.c b/src/pacman/sighandler.c
index 46e6481f..59aaa61f 100644
--- a/src/pacman/sighandler.c
+++ b/src/pacman/sighandler.c
@@ -27,6 +27,9 @@ 
 #include "sighandler.h"
 #include "util.h"
 
+/* the same ANSI sequence as used in console_show_cursor */
+#define SHOW_CURSOR_ANSI "\x1B[?25h"
+
 /** Write function that correctly handles EINTR.
  */
 static ssize_t xwrite(int fd, const void *buf, size_t count)
@@ -60,6 +63,7 @@  static void soft_interrupt_handler(int signum)
 		const char msg[] = "\nHangup signal received\n";
 		xwrite(STDERR_FILENO, msg, ARRAYSIZE(msg) - 1);
 	}
+	xwrite(STDOUT_FILENO, SHOW_CURSOR_ANSI, sizeof(SHOW_CURSOR_ANSI) - 1);
 	if(alpm_trans_interrupt(config->handle) == 0) {
 		/* a transaction is being interrupted, don't exit pacman yet. */
 		return;
@@ -95,6 +99,7 @@  static void segv_handler(int signum)
 	const char msg[] = "\nerror: segmentation fault\n"
 		"Please submit a full bug report with --debug if appropriate.\n";
 	xwrite(STDERR_FILENO, msg, sizeof(msg) - 1);
+	xwrite(STDOUT_FILENO, SHOW_CURSOR_ANSI, sizeof(SHOW_CURSOR_ANSI) - 1);
 
 	/* restore the default handler */
 	_reset_handler(signum);
diff --git a/src/pacman/util.c b/src/pacman/util.c
index a640ffb4..7dfbdbca 100644
--- a/src/pacman/util.c
+++ b/src/pacman/util.c
@@ -1391,6 +1391,15 @@  static int multiselect_parse(char *array, int count, char *response)
 	return 0;
 }
 
+char *safe_fgets_stdin(char *s, int size)
+{
+	char *result;
+	console_show_cursor();
+	result = safe_fgets(s, size, stdin);
+	console_hide_cursor();
+	return result;
+}
+
 int multiselect_question(char *array, int count)
 {
 	char *response, *lastchar;
@@ -1427,7 +1436,7 @@  int multiselect_question(char *array, int count)
 
 		flush_term_input(fileno(stdin));
 
-		if(safe_fgets(response, response_len, stdin)) {
+		if(safe_fgets_stdin(response, response_len)) {
 			const size_t response_incr = 64;
 			size_t len;
 			/* handle buffer not being large enough to read full line case */
@@ -1443,8 +1452,8 @@  int multiselect_question(char *array, int count)
 				lastchar = response + response_len - 1;
 				/* sentinel byte */
 				*lastchar = 1;
-				if(safe_fgets(response + response_len - response_incr - 1,
-							response_incr + 1, stdin) == 0) {
+				if(safe_fgets_stdin(response + response_len - response_incr - 1,
+							response_incr + 1) == 0) {
 					free(response);
 					return -1;
 				}
@@ -1494,7 +1503,7 @@  int select_question(int count)
 
 		flush_term_input(fileno(stdin));
 
-		if(safe_fgets(response, sizeof(response), stdin)) {
+		if(safe_fgets_stdin(response, sizeof(response))) {
 			size_t len = strtrim(response);
 			if(len > 0) {
 				int n;
@@ -1582,7 +1591,7 @@  static int question(short preset, const char *format, va_list args)
 
 	flush_term_input(fd_in);
 
-	if(safe_fgets(response, sizeof(response), stdin)) {
+	if(safe_fgets_stdin(response, sizeof(response))) {
 		size_t len = strtrim(response);
 		if(len == 0) {
 			return preset;
diff --git a/src/pacman/util.h b/src/pacman/util.h
index 2cee479f..fbc5f33c 100644
--- a/src/pacman/util.h
+++ b/src/pacman/util.h
@@ -77,6 +77,7 @@  int colon_printf(const char *format, ...) __attribute__((format(printf, 1, 2)));
 int yesno(const char *format, ...) __attribute__((format(printf, 1, 2)));
 int noyes(const char *format, ...) __attribute__((format(printf, 1, 2)));
 char *arg_to_string(int argc, char *argv[]);
+char *safe_fgets_stdin(char *s, int size);
 
 int pm_printf(alpm_loglevel_t level, const char *format, ...) __attribute__((format(printf,2,3)));
 int pm_asprintf(char **string, const char *format, ...) __attribute__((format(printf,2,3)));