From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 1844 invoked by alias); 5 Mar 2017 08:32:07 -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: X-Seq: 40732 Received: (qmail 430 invoked from network); 5 Mar 2017 08:32:07 -0000 X-Qmail-Scanner-Diagnostics: from out4-smtp.messagingengine.com by f.primenet.com.au (envelope-from , uid 7791) with qmail-scanner-2.11 (clamdscan: 0.99.2/21882. spamassassin: 3.4.1. Clear:RC:0(66.111.4.28):SA:0(-0.7/5.0):. Processed in 1.731883 secs); 05 Mar 2017 08:32:07 -0000 X-Spam-Checker-Version: SpamAssassin 3.4.1 (2015-04-28) on f.primenet.com.au X-Spam-Level: X-Spam-Status: No, score=-0.7 required=5.0 tests=RCVD_IN_DNSWL_LOW, RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL,T_DKIM_INVALID autolearn=unavailable autolearn_force=no version=3.4.1 X-Envelope-From: d.s@daniel.shahaf.name X-Qmail-Scanner-Mime-Attachments: | X-Qmail-Scanner-Zip-Files: | Received-SPF: none (ns1.primenet.com.au: domain at daniel.shahaf.name does not designate permitted sender hosts) DKIM-Signature: v=1; a=rsa-sha1; c=relaxed/relaxed; d= daniel.shahaf.name; h=cc:content-transfer-encoding:content-type :date:from:in-reply-to:message-id:mime-version:references :subject:to:x-me-sender:x-me-sender:x-sasl-enc:x-sasl-enc; s= mesmtp; bh=zXqtahR9TqScDBr8Vr32kdm4kCg=; b=QkOCCEoV3Q+Su6W1fz+Hc JM4fUV1hkwLm4PzLnFbasb+iFv9nvhlgmIudYAibxq7pdebN3TUzQdAF7sjOJtMe gQvnbURAZAkYMtTPX25/2n/BvVpM9AjcAXmeDN054e9PgejvZbY3rBULGIB2qY7Y 6iwCkxZ2/TWeLdc3HZ+q8g= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed/relaxed; d= messagingengine.com; h=cc:content-transfer-encoding:content-type :date:from:in-reply-to:message-id:mime-version:references :subject:to:x-me-sender:x-me-sender:x-sasl-enc:x-sasl-enc; s= smtpout; bh=zXqtahR9TqScDBr8Vr32kdm4kCg=; b=c6J+yMFI8lWeA2xZCdpb 16O6kYlnUuPFGMjPPJqr+/mxcXf7XQKnmIPUKhjoW7NL9+9jCdySUl7CmS+ZJGGZ 5TiBT4vZyXqHdUW35H5kz8lRbs5jAOC3wEfxDwTJPeXm1imVSf5hNDDOhIIYFtb1 0fjsz4GcePBjT38SZA4/OAo= X-ME-Sender: X-Sasl-enc: jLsXDHgbxwcIim2LuHcCg75og2p3WPby4A9puXEpTiOX 1488702718 Date: Sun, 5 Mar 2017 08:27:15 +0000 From: Daniel Shahaf To: Jeremy Cantrell Cc: zsh-workers@zsh.org Subject: Re: patch for ssh completion Message-ID: <20170305082715.GA4879@fujitsu.shahaf.local2> References: <20170302121229.GC6315@fujitsu.shahaf.local2> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20170302121229.GC6315@fujitsu.shahaf.local2> User-Agent: Mutt/1.5.23 (2014-03-12) Jeremy — have you seen my review (quoted below)? Would you be able to revise the patch to address these points, at least the first two? Process-wise, patches that apply to latest master are preferred, but _ssh hasn't been changed since 5.3.1 so that's okay. Cheers, Daniel Daniel Shahaf wrote on Thu, Mar 02, 2017 at 12:12:29 +0000: > Jeremy Cantrell wrote on Tue, Feb 28, 2017 at 11:17:29 -0800: > > Hello! > > > > I've found a tiny problem with the ssh completion script where any host > > entries defined in included configuration files do not appear as choices > > when completing. I've attached a patch that fixes it for me. Below is an > > example that illustrates the problem: > > > > ~/.ssh/config: > > Include hosts > > > > ~/.ssh/hosts: > > Host example > > HostName example.local > > From ssh_config(5): > > Include > Include the specified configuration file(s). Multiple pathnames > may be specified and each pathname may contain glob(3) wildcards > and, for user configurations, shell-like ‘~’ references to user > home directories. Files without absolute paths are assumed to be > in ~/.ssh if included in a user configuration file or /etc/ssh if > included from the system configuration file. Include directive > may appear inside a Match or Host block to perform conditional > inclusion. > > > Thanks, > > Jeremy > > > --- /usr/share/zsh/functions/Completion/Unix/_ssh 2016-12-22 11:27:00.000000000 -0800 > > +++ packages/base/.local/share/zsh/functions/_ssh 2017-02-26 22:37:47.513422235 -0800 > > @@ -681,16 +681,21 @@ > > fi > > if [[ -r $config ]]; then > > local key hosts host > > + local filename configs=($config) > > + grep '^Include\b' "$config" | sed 's/\s\+/ /g' | cut -d' ' -f2 | > > + while read -r filename; do > > + config=$HOME/.ssh/$filename > > This handles your example, but not the other possibilities documented in > the man page, e.g., absolute paths. I expect using those would cause > errors on stderr. (I can't test this since my ssh isn't new enough.) > > The «\b» in grep is not in POSIX. It could be changed to match > whitespace explicitly. I assume we'll also want to use builtin > functionality such as ${(M)…:#Include*} instead of external executables. > > Can filenames be quoted? I.e., does «Include "foo"» include ~/.ssh/foo > or ~/.ssh/\"foo\"? > > Can Include directives be nested? If they can, it'd be nice to handle > that. (But I don't think that's a blocker to accepting the patch) > > > while IFS=$'=\t ' read -r key hosts; do > > if [[ "$key" == (#i)host ]]; then > > for host in ${(z)hosts}; do > > case $host in > > (*[*?]*) ;; > > (*) config_hosts+=("$host") ;; > > esac > > done > > fi > > done < "$config" > > + done > > Thanks for the patch. > > Cheers, > > Daniel