Tests of interrupting completion, and completion_nostat_dirs

Tests of interrupting completion, and completion_nostat_dirs

[Bart Schaefer]

[> workers]

On Jan 29, 11:29am, Greg Klanderman wrote:
} Subject: Re: [Feature suggestion] (user configurable) timeout for generati
}
} >>>>> On January 25, 2014 Bart Schaefer <schaefer@brasslantern.com> wrote:
} 
} > On Jan 24,  9:46am, Greg Klanderman wrote:
} > }
} > } For slow completion due to calling out to a separate process for
} > } completions (_git comes to mind) if that is not currently
} > } interruptible it seems like there might be some hope of adding a
} > } timeout or ensuring C-c will interrupt.
} 
} > This is the thing I had a hard time reproducing.  Even if I force
} > the completion function to be very busy with a loop, I can interrupt
} > it.  (Oddly the value of $? is always 0 in _main_complete after I
} > hit ^C, so I can't detect the signal and issue a message.)

Incidentally, here's something I added to _main_complete to make this
easier to test:

------------
diff --git a/Completion/Base/Core/_main_complete b/Completion/Base/Core/_main_complete
index d6831b8..036f1d3 100644
--- a/Completion/Base/Core/_main_complete
+++ b/Completion/Base/Core/_main_complete
@@ -126,6 +126,11 @@ fi
 
 _completer_num=1
 
+# We assume localtraps to be in effect here ...
+integer SECONDS=0
+trap 'zle -M "Killed by signal in ${funcstack[0]} after ${SECONDS}s";
+      zle -R; return 130' INT QUIT
+
 # Call the pre-functions.
 
 funcs=( "$compprefuncs[@]" )
------------

Any objection to that being pushed to the master git?

} Did you try a busy completion function, or external process?

I tried

    _slow() { sleep 30; return 1 }

and the infinite loop

    _slow() {
        while :; do read -E -k 1 -u 0 </dev/zero >/dev/null; done
        return 1
    }

and even

    _slow() {
        SECONDS=0
        while (( SECONDS < 30 )); do : ; done
        return 1
    }

and I was able to interrupt all of them.

} In the original post it seemed that the slow call was probably reading
} a large directory on a slow NFS mount.

Yes, but both that and stat of an automount are going to be disk-wait
blocking accesses to NFS.
 
} I'll attach the patch below, if you'd be willing to incorporate
} something like it let me know, I'd certainly prefer to have it part of
} zsh.  I would add documentation of course.

Hmm, to include this I'd probably want completion_nostat_dirs to be an
autoloaded parameter in the zsh/complete module or something like that.
It doesn't have any of those right now (the magic parameters that are
only valid inside completion widgets are a somewhat different beast) but
there's no reason it couldn't.

On the other hand ZLE_RPROMPT_INDENT is declared in the main shell even
though it's used only by zsh/zle ... anybody else want to weigh in?

Re: Tests of interrupting completion, and completion_nostat_dirs

[Peter Stephenson]

On Wed, 29 Jan 2014 18:12:06 -0800
Bart Schaefer <schaefer@brasslantern.com> wrote:
> } I'll attach the patch below, if you'd be willing to incorporate
> } something like it let me know, I'd certainly prefer to have it part of
> } zsh.  I would add documentation of course.
> 
> Hmm, to include this I'd probably want completion_nostat_dirs to be an
> autoloaded parameter in the zsh/complete module or something like that.
> It doesn't have any of those right now (the magic parameters that are
> only valid inside completion widgets are a somewhat different beast) but
> there's no reason it couldn't.

This has traditionally been done using something that's only seen by
completion that gets passed in from above.  It could, for example, be
something in compstate set at the top level of _main_complete, but there
are other mechanisms: the menu completion library uses its own
parameters that are set by styles, and sometimes the effect of styles
get passed into builtins which probably doesn't fit here.  A style would
certainly fit better with the rest of the completion system.

This is different from ZLE which doesn't use styles so gets configured
by straight parameters --- that's not ideal; it would be better to use
some form of namespace if we could.

pws

Re: Tests of interrupting completion, and completion_nostat_dirs

[Bart Schaefer]

On Jan 30,  9:30am, Peter Stephenson wrote:
} Subject: Re: Tests of interrupting completion, and completion_nostat_dirs
}
} On Wed, 29 Jan 2014 18:12:06 -0800
} Bart Schaefer <schaefer@brasslantern.com> wrote:
} > Hmm, to include this I'd probably want completion_nostat_dirs to be an
} > autoloaded parameter in the zsh/complete module or something like that.
} > It doesn't have any of those right now (the magic parameters that are
} > only valid inside completion widgets are a somewhat different beast) but
} > there's no reason it couldn't.
} 
} This has traditionally been done using something that's only seen by
} completion that gets passed in from above.

Yes, but in this case there's a corresponding C code change to recognize
the setting.

} It could, for example, be something in compstate set at the top level
} of _main_complete, but there are other mechanisms: the menu completion
} library uses its own parameters that are set by styles, and sometimes
} the effect of styles get passed into builtins which probably doesn't
} fit here.

I suppose the parameter *could* in fact be valid only when inside a
completion widget, and it would then be up to _main_complete to copy
from a zstyle into the parameter so that ztat() could see it.  That
would be roughly the same as the treatment of region_highlight by zle;
it doesn't exist except while doing its job.

Of course that would mean that compctl and/or widgets entered directly
without going through _main_complete would need to do their own zstyle
lookup.  This is the reason that e.g. MENUSCROLL is a global parameter
as well as being copied from a zstyle.

} This is different from ZLE which doesn't use styles so gets configured
} by straight parameters --- that's not ideal; it would be better to use
} some form of namespace if we could.

Right, but the completion-only scoping could stand in for that in the
case of completion.  The "problem" for zle is that, unless we actually
make the internals aware of zstyle values at the C level, there's no
guaranteed entry point where a zstyle can be referenced by shell code.
(zle-line-init could serve that role now, I suppose, but then everyone
needs a default zle-line-init [compare MENUSCROLL above].)

Re: Tests of interrupting completion, and completion_nostat_dirs

[Greg Klanderman]

>>>>> On January 30, 2014 Bart Schaefer <schaefer@brasslantern.com> wrote:

> Right, but the completion-only scoping could stand in for that in the
> case of completion.  The "problem" for zle is that, unless we actually
> make the internals aware of zstyle values at the C level, there's no
> guaranteed entry point where a zstyle can be referenced by shell code.
> (zle-line-init could serve that role now, I suppose, but then everyone
> needs a default zle-line-init [compare MENUSCROLL above].)

Hi Bart and Peter, not sure if you've converged on an appropriate way
to configure the nostat dirs, but I'm happy to rework that part to use
a zstyle or whatever is best.  The way I did it so far was most
expedient for my local patch but I agree a zstyle would probably be
best, though there is the problem of catching all the entry points to
C which can end up calling ztat() as you note..

Greg

Re: Tests of interrupting completion, and completion_nostat_dirs

[Bart Schaefer]

On Feb 3, 10:04am, Greg Klanderman wrote:
}
} Hi Bart and Peter, not sure if you've converged on an appropriate way
} to configure the nostat dirs, but I'm happy to rework that part to use
} a zstyle or whatever is best.

Greg, I'm going to let Peter make the call on this one because he has
become more concerned about namespace clutter that I.

I would say that the change to ztat() should continue to read from a
variable.  I think I'd have ztat() call getaparam() rather than declare
a C array tied to a special variable (compare the way zle_refresh.c
handles zle_highlight) but I'll stop at that.

Re: Tests of interrupting completion, and completion_nostat_dirs

[Greg Klanderman]

>>>>> On February 5, 2014 Bart Schaefer <schaefer@brasslantern.com> wrote:

> Greg, I'm going to let Peter make the call on this one because he has
> become more concerned about namespace clutter that I.

> I would say that the change to ztat() should continue to read from a
> variable.  I think I'd have ztat() call getaparam() rather than declare
> a C array tied to a special variable (compare the way zle_refresh.c
> handles zle_highlight) but I'll stop at that.

Thank you Bart, the only downside I see is that I currently use the
existence of the special variable to decide whether to configure the
automount points in the fake-files zstyle, since I end up having to
kill my shell from another window if the automounts get configured in
a shell that doesn't have my patch and I ever hit tab to complete
under an automount.

If I use getaparam, is the only way to test if the feature exists with
a ZSH_VERSION / ZSH_PATCHLEVEL check?  Do we have any mechanism for
creating named features to test against?  Would it make sense to add a
readonly 'features' associative array (maybe in the zsh/parameter module)
whose keys are feature strings?

I'll wait to hear from Peter before doing anything..

Greg

Re: Tests of interrupting completion, and completion_nostat_dirs

[Peter Stephenson]

On Wed, 05 Feb 2014 08:56:31 -0500
Greg Klanderman <gak@klanderman.net> wrote:
> I'll wait to hear from Peter before doing anything..

Looks like the right answer is to add the parameter within
complete.c.  As it's an array, it's going to have to go separately
into the comprparams list (since we can't have arrays inside hashes at
the moment), but that means you can manipulate it from the
shell just as before (and the semantics for dealing with it internally
are exactly what you've got, except it's only visible in the completion
functions because you've added it to a different parameter structure).

Then _main_complete (or anything further down that knows when the
value is needed, but _main_complete is fine) can retrieve an
appropriately named style directly into the array, which is a single
line change.  So I think this is all straightforward.

This then looks like other completion configuration --- only the styles
are visible outside the completion system itself.

pws

Re: Tests of interrupting completion, and completion_nostat_dirs

[Bart Schaefer]

On Feb 5,  8:56am, Greg Klanderman wrote:
}
} >>>>> On February 5, 2014 Bart Schaefer <schaefer@brasslantern.com> wrote:
} 
} > I think I'd have ztat() call getaparam() rather than declare
} > a C array tied to a special variable (compare the way zle_refresh.c
} > handles zle_highlight) but I'll stop at that.
} 
} Thank you Bart, the only downside I see is that I currently use the
} existence of the special variable to decide whether to configure the
} automount points in the fake-files zstyle [...]
} 
} If I use getaparam, is the only way to test if the feature exists with
} a ZSH_VERSION / ZSH_PATCHLEVEL check?  Do we have any mechanism for
} creating named features to test against?

You could declare it as a provided feature in the zsh/complete module,
and test for it with "zmodload -l ...".  I'm not sure whether you can
declare that a module has a "p:" (parameter) feature but not have any
real special parameter to go with it ...

On Feb 5,  2:23pm, Peter Stephenson wrote:
}
} Looks like the right answer is to add the parameter within
} complete.c.

And declare it as a feature to solve Greg's problem above?  If it's only
a special parameter when completion is actually in progress, it can't be
tested from a startup file.

Re: Tests of interrupting completion, and completion_nostat_dirs

[Peter Stephenson]

On Wed, 05 Feb 2014 09:15:01 -0800
Bart Schaefer <schaefer@brasslantern.com> wrote:
> On Feb 5,  2:23pm, Peter Stephenson wrote:
> }
> } Looks like the right answer is to add the parameter within
> } complete.c.
> 
> And declare it as a feature to solve Greg's problem above?  If it's only
> a special parameter when completion is actually in progress, it can't be
> tested from a startup file.

Hmmm... parameter features are handled automatically, so the parameter
would be added permanently.  However, there is the option of adding
non-standard (looks like I called them "abstract") features that aren't
handled automatically by the feature mechanism, so we could add that
(and make it do nothing when enabled or disabled).

Really, this is asking for something a bit more lightweight within the
feature mechanism, i.e. a set of flags that you can query but not turn
on or off.

But having something in the main shell to test for a completion feature
doesn't sound the right solution, so we should probably do something
like this.

(The other way would be to use the zstyle -e option to execute some code
when the style is requested, which would ensure all the requirements are
satisfied.)

pws

Re: Tests of interrupting completion, and completion_nostat_dirs

[Greg Klanderman]

OK, thank you guys, gives me some stuff to look into and get
started.. am a little busy right now, but I'll try to find
some time to do this and get it submitted.

thank you
Greg