From: Matthew Martin <phy1729@gmail.com>
To: zsh-workers@zsh.org
Subject: Re: [PATCH] _pick_variant: Update builtin check
Date: Wed, 20 Mar 2019 07:52:39 -0500 [thread overview]
Message-ID: <20190320125238.GA48465@CptOrmolo.darkstar> (raw)
In-Reply-To: <20190320033815.GA22718@CptOrmolo.darkstar>
On Tue, Mar 19, 2019 at 10:38:15PM -0500, Matthew Martin wrote:
> On Tue, Mar 19, 2019 at 09:05:13PM -0500, Matthew Martin wrote:
> > Move the builtin check prior to the cache lookup and don't cache
> > builtins if -b is provided. This corrects the result if a user uses both
> > a builtin and external version of a command in one shell session.
> >
> > The command may be builtin only if all precommands are
> > builtin-preserving which are enumerated in builtin_precommands.
> > Since a builtin-preserving precommand must be itself builtin, I figure
> > it's easier to maintain as a white list. This corrects the case Daniel
> > brought up: command chmod <tab>.
>
> I've realized that I was only testing on a system that uses the fallback
> variant and the patch will give incorrect results on GNU systems since
> _call_program may call a builtin. I'll rework the patch (although I'm
> open to suggestions for fixing it).
There are four booleans in play:
- If command (or a non-builtin-preserving precommand) is specified
(${#precommands:|builtin_precommands})
- If builtin is specified ($+precommands[(r)builtin])
- If -b is passed to _pick_variant ($+opts[-b])
- If the command is a builtin ($+builtins[$opts[-c]])
If command is specified, set pre=command to force _call_program to run
the command. Eligible for caching.
If -b is passed and (builtin is specified or the command is a builtin),
return the -b variant. Ineligible for caching.
If builtin is specified, set pre=builtin to force _call_program to run
the builtin. Ineligible for caching.
This leaves 3 cases:
- command not specified, builtin not specified, -b passed, is not builtin
- command not specified, builtin not specified, -b not passed, is not builtin
- command not specified, builtin not specified, -b not passed, is builtin
In all cases, fall through to _call_program with no prefix set.
In the first two cases, the command is not a builtin, so they are
clearly caching eligible. In the last case, I'm unsure if caching should
be allowed, but one would hope there are few builtins that have
a completer that uses _pick_variant and does not pass -b. Since this
case should be rare, I've omitted it from the cache eligibility check.
- Matthew Martin
---
Completion/Base/Utility/_pick_variant | 28 ++++++++++++++++++---------
Completion/Zsh/Command/_builtin | 2 ++
2 files changed, 21 insertions(+), 9 deletions(-)
diff --git a/Completion/Base/Utility/_pick_variant b/Completion/Base/Utility/_pick_variant
index 9099e3599..22faa8381 100644
--- a/Completion/Base/Utility/_pick_variant
+++ b/Completion/Base/Utility/_pick_variant
@@ -1,9 +1,11 @@
#autoload
-local output cmd pat
-local -a var
+local output cmd pat pre
+local -a builtin_precommands var
local -A opts
+builtin_precommands=(- builtin eval exec nocorrect noglob time)
+
(( $+_cmd_variant )) || typeset -gA _cmd_variant
zparseopts -D -A opts b: c: r:
@@ -13,19 +15,27 @@ while [[ $1 = *=* ]]; do
var+=( "${1%%\=*}" "${1#*=}" )
shift
done
-if (( $+_cmd_variant[$opts[-c]] )); then
- (( $+opts[-r] )) && eval "${opts[-r]}=${_cmd_variant[$opts[-c]]}"
- [[ $_cmd_variant[$opts[-c]] = "$1" ]] && return 1
+
+if (( ${#precommands:|builtin_precommands} )); then
+ pre=command
+elif (( $+opts[-b] && ( $precommands[(r)builtin] || $+builtins[$opts[-c]] ) )); then
+ (( $+opts[-r] )) && eval "${opts[-r]}=$opts[-b]"
return 0
+elif (( $precommands[(r)builtin] )); then
+ pre=builtin
+else
+ # Neither builtin nor command-forcing precommand specified,
+ # so no prefix is needed
+ pre=
fi
-if [[ $+opts[-b] -eq 1 && -n $builtins[$opts[-c]] ]]; then
- _cmd_variant[$opts[-c]]=$opts[-b]
+if [[ $pre != builtin ]] && (( $+_cmd_variant[$opts[-c]] )); then
(( $+opts[-r] )) && eval "${opts[-r]}=${_cmd_variant[$opts[-c]]}"
+ [[ $_cmd_variant[$opts[-c]] = "$1" ]] && return 1
return 0
fi
-output="$(_call_program variant $opts[-c] "${@[2,-1]}" </dev/null 2>&1)"
+output="$(_call_program variant $pre $opts[-c] "${@[2,-1]}" </dev/null 2>&1)"
for cmd pat in "$var[@]"; do
if [[ $output = *$~pat* ]]; then
@@ -36,6 +46,6 @@ for cmd pat in "$var[@]"; do
done
(( $+opts[-r] )) && eval "${opts[-r]}=$1"
-_cmd_variant[$opts[-c]]="$1"
+[[ $pre != builtin ]] && _cmd_variant[$opts[-c]]="$1"
return 1
diff --git a/Completion/Zsh/Command/_builtin b/Completion/Zsh/Command/_builtin
index 9fb6acf7b..a77af9879 100644
--- a/Completion/Zsh/Command/_builtin
+++ b/Completion/Zsh/Command/_builtin
@@ -1,5 +1,7 @@
#compdef builtin
+precommands+=(builtin)
+
if (( $CURRENT > 2 )); then
shift words
(( CURRENT -- ))
--
2.21.0
next prev parent reply other threads:[~2019-03-20 12:53 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-03-20 2:05 Matthew Martin
2019-03-20 3:38 ` Matthew Martin
2019-03-20 12:52 ` Matthew Martin [this message]
2019-03-20 13:05 ` Daniel Shahaf
2019-03-21 0:16 ` Matthew Martin
2019-03-21 11:57 ` Daniel Shahaf
2019-03-21 12:40 ` Matthew Martin
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20190320125238.GA48465@CptOrmolo.darkstar \
--to=phy1729@gmail.com \
--cc=zsh-workers@zsh.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://git.vuxu.org/mirror/zsh/
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).