Re: Interrupting globs (Re: Something rotten in tar completion)

[Mikael Magnusson <mikachu@xxxxxxxxx>]

On Fri, Dec 5, 2014 at 11:07 PM, Peter Stephenson
<p.w.stephenson@xxxxxxxxxxxx> wrote:
> On Fri, 5 Dec 2014 20:34:17 +0000
> Peter Stephenson <p.w.stephenson@xxxxxxxxxxxx> wrote:
>> On the other problem I came up with, that eval is resetting errflag even
>> if you've interrupted: how about the following?  Add a bit to errflag to
>> signify that the user interrupted the shell rather than that some
>> internal error (e.g. syntax) occurred.  Only reset this new bit in a few
>> key places: the main command loop when executing, the top of ZLE when
>> editing being the obvious places.  Convert other "errflag = 0"
>> assignments case by case so that they just remove bit 0; then eval can
>> continue to do its job of acting as a sandbox but without screwing up
>> the behaviour of interrupts.  I think doing that is fairly mechanical
>> and it achieves what's needed without compromising anything else.
>
> Here's my first go; it does seem to do what I want, and as a by-product
> fixes all the little race conditions we've always had that meant you
> couldn't interrupt chunks of code that were executed in any kind of
> sandbox because the condition got reset afterwards.  I think a few of
> these have been annoying me over the years.
>
> The general strategy is to use the bit ERRFLAG_ERROR for internal
> failures and ERRFLAG_INT for user interrupts.  There are only two
> cases of the latter: on an untrapped SIGINT, the obvious case, and also
> on a trapped SIGINT or SIGQUIT where we've been told to behave as if the
> trap didn't trap the error condition.  That's straightforward for
> SIGINT, less so for SIGQUIT but I took my cue from the fact that Bart
> thought it worthwhile trapping SIGQUIT as an interactive "no, I really
> mean abort" in completion, which implies that if we trap it we want it
> to work at least as well as SIGINT.
>
> Correspondingly, most of the time only the ERRFLAG_ERROR bit gets
> reset.  ERRFLAG_INT gets reset only in the following cases:
>
> - in the main command loop.  This is what causes the shell not to exit but
> instead go back to the main command loop when you ^C a command.
>
> - at the start of zleread, so we can read the next thing to do whatever
> just happened.  I'm not sure this is particularly useful since
> in this case you'd typically expect the previous condition to have
> occurred and it could mean e.g. you ignore an interrupt that
> occurred just before a "vared".
>
> - when we just finished completion.  This is needed so that the cases
> that got this whole business kicked off behave as now (but more
> reliably) --- a ^C gets you back to the command line, but the command
> line is not trashed as it would be if you ^Ced outside completion (try
> it if you're confused).  There's a race here, but it's no worse than it
> ever was.
>
> To ensure ERRFLAG_INT doesn't get reset unnecessarily there are a number
> of cases where restoring errflag to a previously saved value keeps the
> ERRFLAG_INT bit if it got set in the meanwhile.  I hope the rationale
> here is obvious --- the ERRFLAG_ERROR is an internal state that needs
> resetting, the ERRFLAG_INT an asynchronous condition where the user
> doesn't care what the internal state is.
>
> By the way, looking at the patch below you might wonder if it wouldn't
> be more efficient to add a separate flag for interrupt error conditions
> to test.  It wouldn't --- there are many more cases where errflag is
> tested than when it is set, not affected by the patch below.
>
> I suspect we'll just have to try this out and see how it works.

This seems to work well for me in the cases you talked about, but I
quickly noticed one surprising problem. I have some stuff in my
chpwd() hook to show git branches and stuff, and these used to be
interruptible by ctrl-c (the commands are very fast with hot cache,
but can be somewhat painful with cold cache, like 5-10 seconds delay).
With the patch, I cannot interrupt them (sometimes?).

chpwd () {
    stty -echo >&/dev/null
    test -f .tdldb && tdll -1 >&2
    test -d .git && {
        git branch
        test -d .git/svn && {
            echo -n r
            git svn find-rev master
        }
        git name-rev HEAD
        git describe --tags HEAD 2> /dev/null
    } >&2
    if [[ "$_NONOCDLS" = 1 ]]
    then
        ls $LS_OPTIONS >&2
    fi
}

The git branch is actually git brunch, which is git aliased to
brunch  -- alias for '!git branch -v|sed -e 's/t/ /g' -e 's/(.{'$((
$(tput cols) - 1 ))'})
There's no .tdldb in that directory either, but including it unmodified anyway.

-- 
Mikael Magnusson