On 22-06-28 03:32pm, Omar Polo wrote:
> 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 ;)

It definitely does and I think your suggestion is the right way to do
this; the other way just speaks to my laziness :)

> > 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
> > @@ -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.

Sorry I missed this. I don't like making the same mistake twice and
you've already told me about this. You told me about man -Tlint too and
I have been using it but it didn't pick this one up.

> >  	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 :)

I made the change because I thought it would be better that invalid
values fallback to the default value tbh, but I'll defer to your
judgement; if you think it's best this way, I'm happy to make the
change? If so, should we also document that with something like:

  Invalid values will disable the timeout as if TOG_KEYMAP_TIMEOUT was
  defined with 0.


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