wryun / es-shell

es: a shell with higher-order functions
http://wryun.github.io/es-shell/
Other
313 stars 26 forks source link

Fix ^\ handling with readline, without breaking ^D handling with readline. #116

Closed jpco closed 1 month ago

jpco commented 2 months ago

(Re-)fixes #111. The problem with the first fix attempt in #113 is that it changed the code that calls callreadline() from thinking a NULL return value always means EOF to thinking it never does. In fact, it sometimes does! So this version is a little more nuanced, only setting errno = EINTR in callreadline() when a signal has interrupted things.

I will be less hasty this time and not submit this myself, even though I think it's a fairly uncontroversial bug fix ("surely this time it'll work...")

As an aside, I kind of think /signame should actually just SIG_IGN the signal in the local process and re-set it to SIG_DFL in setsigdefaults(). Seems like the most straightforward thing, and no more racey than the other signal behaviors already being handled by setsigdefaults(). It also (seems to) match bash and tcsh on my machine, but interestingly shells seem pretty diverse in what they actually do on signals at the prompt.

jpco commented 2 months ago

Incredible. Almost this exact same fix was posted on the rc mailing list just over 30 years ago: https://inbox.vuxu.org/rc-list/9405101358.aa24953@ceres.srg.af.mil/

wryun commented 2 months ago

If I were you, I'd consider that the same fix is in rc an endorsement... reminds me of my earlier dreams: https://github.com/wryun/es-shell/issues/1

jpco commented 1 month ago

I've been running with this for the last few days and it seems to work just fine. Between that and the vote of confidence from 1994, I feel good about merging as-is.