From: Omar Polo <op@omarpolo.com>
Subject: Re: tog: user-defined keymap timeout
Cc: gameoftrees@openbsd.org
Date: Tue, 28 Jun 2022 15:32:02 +0200

Mark Jamsek <mark@jamsek.com> wrote:
> On 22-06-28 10:47am, Omar Polo wrote:
> > [...]
> > random thought: maybe we can allow 0 to mean "no delay"?
> 
> Done! And tbh I first thought this was redundant as we could just set it
> to 255 to achieve the same thing in practice. But having tested this
> just now, I really like it :)

wops, haven't thought about it.  well, probably TOG_KEYMAP_TIMEOUT=0
reads better than 255 ;)

> Updated diff:

two small nits remains, sorry for not noticing them before, then it's ok
for me :)

> diff refs/heads/main refs/heads/dev/nkeymap
> blob - 728ba5ee9e20edbde20ed7f0c2ff1cbabad89c0b
> blob + acc4a879396877352bf8cbabcba31222cb07048a
> --- tog/tog.1
> +++ tog/tog.1
> @@ -60,10 +60,16 @@ denoted by N in the descriptions below, and is used as
>  operation as indicated.
>  When the first integer for a count modifier is entered,
>  .Nm
> -will wait 500 milliseconds for each successive integer or the compound sequence
> +will wait 1 second for each successive integer or the compound sequence
>  to complete.
>  If this sequence should timeout or does not conclude with a valid key binding,
>  the command is aborted and any preceding count is reset.
> +This behavior can be customized with the
> +.Ev TOG_KEYMAP_TIMEOUT
> +environment variable.
> +See
> +.Sx ENVIRONMENT
> +for details.
>  The global key bindings are:
>  .Bl -tag -width Ds
>  .It Cm Q
> @@ -561,6 +567,14 @@ work tree, use the repository path associated with thi
>  .El
>  .Sh ENVIRONMENT
>  .Bl -tag -width TOG_COLORS
> +.It Ev TOG_KEYMAP_TIMEOUT
> +Define the timeout interval between compound sequence keypresses.
> +The value must be an integer between 0 and 255, and specifies how many tenths
> +of a second
> +.Nm
> +waits before timing out. 0 disables timeout so that

new sentence, new line.

> +.Nm
> +waits until a non-numeric key is entered (default: 10).
>  .It Ev TOG_COLORS
>  .Nm
>  shows colorized output if this variable is set to a non-empty value.
> blob - 87eeaf0b99951e273aa30da21d81e7ab116412c7
> blob + b7db261c67a024d7918645c7d8637940e65ce369
> --- tog/tog.c
> +++ tog/tog.c
> @@ -596,6 +596,8 @@ static const struct got_error *close_ref_view(struct t
>  static const struct got_error *search_start_ref_view(struct tog_view *);
>  static const struct got_error *search_next_ref_view(struct tog_view *);
> 
> +static int tog_timeout = 10;	/* timeout for compound keymaps (default: 1s) */
> +
>  static volatile sig_atomic_t tog_sigwinch_received;
>  static volatile sig_atomic_t tog_sigpipe_received;
>  static volatile sig_atomic_t tog_sigcont_received;
> @@ -888,10 +890,10 @@ view_search_start(struct tog_view *view)
>  }
> 
>  /*
> - * Compute view->count from numeric user input.  User has five-tenths of a
> - * second to follow each numeric keypress with another number to form count.
> + * Compute view->count from numeric user input.  User has tog_timeout tenths of
> + * a second to follow each numeric keypress with another number to form count.
> + * If tog_timeout is 0, block till non-numeric input is entered.
>   * Return first non-numeric input or ERR and assign total to view->count.
> - * XXX Should we add support for user-defined timeout?
>   */
>  static int
>  get_compound_key(struct tog_view *view, int c)
> @@ -899,7 +901,10 @@ get_compound_key(struct tog_view *view, int c)
>  	int x, n = 0;
> 
>  	view->count = 0;
> -	halfdelay(5);  /* block for half a second */
> +	if (tog_timeout)
> +		halfdelay(tog_timeout);  /* timeout interval between keys */
> +	else
> +		cbreak();  /* timeout disabled--block till non-numeric input */
>  	wattron(view->window, A_BOLD);
>  	wmove(view->window, view->nlines - 1, 0);
>  	wclrtoeol(view->window);
> @@ -927,6 +932,24 @@ get_compound_key(struct tog_view *view, int c)
>  	return c;
>  }
> 
> +static void
> +set_keymap_timeout(void)
> +{
> +	const char *timeout;
> +
> +	/* fallback to default on error or if not set */
> +	timeout = getenv("TOG_KEYMAP_TIMEOUT");
> +	if (timeout != NULL) {
> +		const char	*errstr;
> +		int		 t;
> +
> +		t = strtonum(timeout, 0, 255, &errstr);
> +		if (!errstr)
> +			tog_timeout = t;

since we're not really interested in handling failure in the conversion
and strtonum returns 0 on error anyway, I think you can simplify this
back to

+	if (timeout != NULL)
+		tog_timeout = strtonum(timeout, 0, 255, NULL);

if you're fine that with an invalid value it disables the timeout.

(and at that point you could even inline this into the main() if you
want :)

> +	}
> +
> +}
> +
>  static const struct got_error *
>  view_input(struct tog_view **new, int *done, struct tog_view *view,
>      struct tog_view_list_head *views)
> @@ -7418,6 +7441,8 @@ main(int argc, char *argv[])
>  		}
>  	}
> 
> +	set_keymap_timeout();
> +
>  	if (cmd == NULL) {
>  		if (argc != 1)
>  			usage(0, 1);