On 22-06-28 10:47am, Omar Polo wrote:
> Hi!
> 
> Mark Jamsek <mark@jamsek.com> wrote:
> > As Stefan suggested in the original thread, enable user-defined timeout
> > for compound keymaps, configurable with an envvar. This also increases
> > the default value to 1 second (up from 0.5s).
> 
> I'm still not sold on the idea, but since many programs (tmux, vi, ...)
> have settings for this maybe we need one too?  i'll leave to other to
> judge :)

I admit, I was somewhat ambivalent about this until today. But for
whatever reason, the 500ms timeout was too fast this afternoon and
I kept missing the map I was going for, which was annoying. So first
I changed the default to 1s, then I thought of Stefan's comments and
realised it was necessary.

> > diff refs/heads/main refs/heads/dev/nkeymap
> > blob - 728ba5ee9e20edbde20ed7f0c2ff1cbabad89c0b
> > blob + 7acca5d60fb3d0dfb62ffe6c1848aa16315f03c3
> > --- tog/tog.1
> > +++ tog/tog.1
> > @@ -64,6 +64,11 @@ will wait 500 milliseconds for each successive integer
>                      ^^^^^^^^^^^^^^^^^^^^^^^^^^
> 
> maybe we need to update that too then? :)

I had a feeling I missed something. I also left a stale comment behind
too :)

> >  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 TOG_KEYMAP_TIMEOUT environment
> 
> please use the Ev macro to mark env variables:
> 
> +This behavior can be customized with the
> +.Ev TOG_KEYMAP_TIMEOUT
> +environment

Done. Thanks, Omar!

> > +variable.
> > +See
> > +.Sx ENVIRONMENT
> > +for details.
> >  The global key bindings are:
> >  .Bl -tag -width Ds
> >  .It Cm Q
> > @@ -561,6 +566,12 @@ 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 1 and 255, and specifies how many tenths
> > +of a second
> > +.Nm
> > +will wait before timing out (default: 10).
> >  .It Ev TOG_COLORS
> >  .Nm
> >  shows colorized output if this variable is set to a non-empty value.
> > blob - 87eeaf0b99951e273aa30da21d81e7ab116412c7
> > blob + 6f9bc97a1dbc436514494bb9e7f79b8ea9e5be02
> > --- tog/tog.c
> > +++ tog/tog.c
> > @@ -504,6 +504,7 @@ struct tog_view {
> >  	int lines, cols; /* copies of LINES and COLS */
> >  	int ch, count; /* current keymap and count prefix */
> >  	int focussed; /* Only set on one parent or child view at a time. */
> > +	int timeout; /* timeout for compound key maps (default: 10=1s) */
> 
> why set this per-view?  shouldn't be easier to have a global for it, set
> only once during startup instead of adapting all the view_* functions?
> Are we expecting to have different timeouts per-view?

I actually started doing this and for some reason I can't explain
I changed it. But, yes, this makes a lot more sense.

> >  	int dying;
> >  	struct tog_view *parent;
> >  	struct tog_view *child;
> > @@ -899,7 +900,7 @@ get_compound_key(struct tog_view *view, int c)
> >  	int x, n = 0;
> > 
> >  	view->count = 0;
> > -	halfdelay(5);  /* block for half a second */
> > +	halfdelay(view->timeout);  /* timeout interval between keys */
> >  	wattron(view->window, A_BOLD);
> >  	wmove(view->window, view->nlines - 1, 0);
> >  	wclrtoeol(view->window);
> > @@ -927,6 +928,20 @@ get_compound_key(struct tog_view *view, int c)
> >  	return c;
> >  }
> > 
> > +static void
> > +set_keymap_timeout(struct tog_view *view)
> > +{
> > +	const char *timeout;
> > +
> > +	/* fallback to default on error or if not set */
> > +	timeout = getenv("TOG_KEYMAP_TIMEOUT");
> > +	if (timeout != NULL)
> > +		view->timeout = strtonum(timeout, 1, 255, NULL);
> 
> 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 :)

Updated diff:

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
+.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;
+	}
+
+}
+
 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);

-- 
Mark Jamsek <fnc.bsdbox.org>
GPG: F2FF 13DE 6A06 C471 CA80  E6E2 2930 DC66 86EE CF68