From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on inbox.vuxu.org X-Spam-Level: X-Spam-Status: No, score=-1.1 required=5.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,FREEMAIL_FROM,MAILING_LIST_MULTI,RCVD_IN_DNSWL_NONE autolearn=ham autolearn_force=no version=3.4.2 Received: from primenet.com.au (ns1.primenet.com.au [203.24.36.2]) by inbox.vuxu.org (OpenSMTPD) with ESMTP id 1fee9dcd for ; Wed, 20 Mar 2019 12:53:35 +0000 (UTC) Received: (qmail 4686 invoked by alias); 20 Mar 2019 12:53:21 -0000 Mailing-List: contact zsh-workers-help@zsh.org; run by ezmlm Precedence: bulk X-No-Archive: yes List-Id: Zsh Workers List List-Post: List-Help: List-Unsubscribe: X-Seq: 44148 Received: (qmail 14133 invoked by uid 1010); 20 Mar 2019 12:53:21 -0000 X-Qmail-Scanner-Diagnostics: from mail-ot1-f66.google.com by f.primenet.com.au (envelope-from , uid 7791) with qmail-scanner-2.11 (clamdscan: 0.101.1/25393. spamassassin: 3.4.2. Clear:RC:0(209.85.210.66):SA:0(-1.7/5.0):. Processed in 4.856466 secs); 20 Mar 2019 12:53:21 -0000 X-Envelope-From: phy1729@gmail.com X-Qmail-Scanner-Mime-Attachments: | X-Qmail-Scanner-Zip-Files: | Received-SPF: pass (ns1.primenet.com.au: SPF record at _netblocks.google.com designates 209.85.210.66 as permitted sender) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=date:from:to:subject:message-id:mail-followup-to:references :mime-version:content-disposition:in-reply-to:user-agent; bh=BqxEKrswBUI0FVpPIxpY6TtLGvkiP0MNHvS8JPOMt1M=; b=CWneaYBa/XM0/ooq5pBiZQVfMMl8nLhmtaLD0oHtfG/825x1k/oPEsNbpkhykvtLDt 92hA0umW/hrKkR6ft9aDHdJsdyI/R8zUGoiPyO3tYv9MUM7pINJTb6Zk8IjXNWNjTy8m +hJ15dOiRKcr1OQjSvDQ86QTQl1+WVwzxIDzte5LCIwMcORZ29K0fT2Fb7FJpnXj8PyJ rL74z1+7yb+weI0nNIxIikCQkJJtxqNgtKM1J/fcNhhDkloWN/du/KExTII3WkwkcKyj dT30fsMkcLh8kz2cSTIjrCYWlVl+ZspnIod0G7CalPgBLtDE0aGyYLH6pdFBX55qkicR +Pyw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:subject:message-id:mail-followup-to :references:mime-version:content-disposition:in-reply-to:user-agent; bh=BqxEKrswBUI0FVpPIxpY6TtLGvkiP0MNHvS8JPOMt1M=; b=MWMhvAQpQuyXggOjJNwjtf04oAl5SbwT/0YGiywV29bhzMG9kjoLjnlq9wUJE2CejO iYv4O8ZByW69xdB7UY2GC+q6PRvZwdKic/3v+KagzMDboHBVyGnW8qkUHSrBq9luScKP zsg9ExNegf6ELs7TbU9gE5pJOeuEHuSJKHfjnxp+MrpKePcFboHwKjGD1nuf1BFszMqm ggFV3YMUsHZgE9dLvbamAe1+yGaJsL8WyQWI1gOq4EDuMJlT31vF7ESlNe5SU3v7fjqE VObEheR9Gyhn5NFe1/WMJ8+BkbFkLgx3MqTw/i3VRiYUPfkrhyQSbHcM2v9j0AJjuAct UaCw== X-Gm-Message-State: APjAAAVV2jBOeDfb/Iqe2AxE+WxSV6DVoNBv4HdPAr0JX971fxEJeXyQ 5eOjbJyvifaIIGvRSHratsQKY0eb X-Google-Smtp-Source: APXvYqyWzgtSVrUHW9/Esqv1sSvUPJp/51quf4XLPwSUYYZhcD5Kld7JaQVIIGwN8U4nCblFB6LRLw== X-Received: by 2002:a9d:6a4a:: with SMTP id h10mr1231704otn.157.1553086361949; Wed, 20 Mar 2019 05:52:41 -0700 (PDT) Date: Wed, 20 Mar 2019 07:52:39 -0500 From: Matthew Martin To: zsh-workers@zsh.org Subject: Re: [PATCH] _pick_variant: Update builtin check Message-ID: <20190320125238.GA48465@CptOrmolo.darkstar> Mail-Followup-To: zsh-workers@zsh.org References: <20190320020511.GA6739@CptOrmolo.darkstar> <20190320033815.GA22718@CptOrmolo.darkstar> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20190320033815.GA22718@CptOrmolo.darkstar> User-Agent: Mutt/1.11.4 (2019-03-13) 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 . > > 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]}" &1)" +output="$(_call_program variant $pre $opts[-c] "${@[2,-1]}" &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