From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 17768 invoked by alias); 19 Mar 2016 01:13: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: 38184 Received: (qmail 29179 invoked from network); 19 Mar 2016 01:13:06 -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=-1.9 required=5.0 tests=BAYES_00,FREEMAIL_FROM, T_DKIM_INVALID autolearn=ham autolearn_force=no version=3.4.1 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc; bh=QkCIp9WRlnhJMZMKuM0kyabX8OgRCApXbaOkbrZrDnk=; b=tYdu34KL5cfQ+s8QTnN3QK3Ecv3qUzfN5ih9kn6yoNZG8bTO6UWBZBpO8dTsBwUsfo NPz8aY/IleJ6dsUZXt6+cndEPDm6IMRBnL5ZqW9dt7YE/TTN9eXcXs74YlataJiUxTpL EPNNsa1faX5noC7eqqt+9xFGRV9AK695BjsI3uvkxc11iMqSkpIcW++Ty0D/G5F5JGVp ZdForO6Tc6TE/ATwS1zQGM50UA7rSWRWBf8/QS1/aYWOFSXwptZWbGmoV4ypc1KpvD1C k0Ju3wwg9LceePILKO4L1bGcRzrkbqMOtWoTjda8Cdpu6X3qF83wrTMXPXS+H9wy0ObV VtsA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:date :message-id:subject:from:to:cc; bh=QkCIp9WRlnhJMZMKuM0kyabX8OgRCApXbaOkbrZrDnk=; b=SPnemebikHPqzJvYSVt3s+sbJ+zSuCdoqba8572q/t0XuOWD4ttJ0sZ/hFGFMabas6 eZYsz9ZBLqaWtTtXvv5rQqGihxE3O7N6LqDAm1RF912qAJlQbDOvcC2V39vAvL9Llqy/ YN4iQvs8RtOapdak2IurjhMkturVA+rZok+Yek7HSgYpitFdIAPAxGXI2Ytd3CcjqvCo gPuFCKDd+dNxvPslp7VsxNkHTttbsuyE1bk9PD9z2sNpL5mU1dlgycoYMoNiynV28ceg tAt4UOgdDblj52Ybzt8fsz6JmVLCea9TYmz9fqHxre2vLdsR7xqKnL09JQUdACg9HQ/H 9i2Q== X-Gm-Message-State: AD7BkJIXWdEYEHljLuclH5En+gOe+erOuubwjJQ6GcgTrvoah50hwOECbFr+aehf1dsZXWskrPk27gj2OauDHw== MIME-Version: 1.0 X-Received: by 10.140.37.113 with SMTP id q104mr25626090qgq.104.1458349982169; Fri, 18 Mar 2016 18:13:02 -0700 (PDT) In-Reply-To: <31107.1458343247@thecus.kiddle.eu> References: <1458317336-27434-1-git-send-email-mikachu@gmail.com> <31107.1458343247@thecus.kiddle.eu> Date: Sat, 19 Mar 2016 02:13:02 +0100 Message-ID: Subject: Re: PATCH: Fix "35531: fallback on file completion" From: Mikael Magnusson To: Oliver Kiddle Cc: zsh workers Content-Type: text/plain; charset=UTF-8 On Sat, Mar 19, 2016 at 12:20 AM, Oliver Kiddle wrote: > Mikael Magnusson wrote: >> The above commit just changed a bunch of stuff without testing, the >> result was that the completer didn't work. It now works, to an extent. > > Specifically, in what way didn't it work? Every completion after "adb anything " only completed files. > Testing will have been limited because I don't have any device adb could > connect to (still using an ancient Nokia) and I don't typically use or > install it. There's a different adb command on Solaris which is perhaps > why I was annoyed at it not falling back to default file completion. > >> The one remaining caveat is that if you enter a subcommand the completer >> doesn't recognize, it will go on to offer subcommands instead of falling >> back to _default. I'm not sure what it's trying to do with the >> "sanitize context" stuff, or "shift words" etc which just makes the rest >> of the code not know where it is, but I don't care enough to rewrite it. > > The "sanitize context" stuff was because it was putting the sub-command > into the wrong component of $curcontext. Completion contexts are > supposed to be: > :completion::::: > > We typically put subcommands into by changing it to > -. If you use a colon instead of a dash, the > subcommand will be in and thereafter, everything gets messed up. > >> -(( $+functions[_adb_check_log_redirect] )) || >> +(( $+functions[adb_check_log_redirect] )) || >> _adb_check_log_redirect () { > >> - LOG_REDIRECT=${$(adb ${=ADB_DEVICE_SPECIFICATION} shell getprop log.redirect-stdio 2>/dev/null)// >> + LOG_REDIRECT=${$(adb ${=ADB_DEVICE_SPECIFICATION} shell getprop log.redirect-stdio)// >> /} > > This should really be using _call_program but I'm sure I redirected > stderr for a reason. It was probably spewing out errors to the terminal > because it couldn't find an attached device. > > Are you sure you want to back that out? No, I removed those hunks from the commit before pushing, included in the sent diff by mistake. > I'm not actually fond of these guards on function definitions myself > because they prevent the function from being reloaded when the > underlying file in $fpath is modified and reloaded. I only use them in > cases where it seems likely that someone would want to override the > function. > > I also don't really like _adb being chatty with lots of _message -r > calls. I tend to prefer completion functions to just do the best they > can of generating matches and otherwise keep quiet. Yeah, there's some really questionable code in there, and lots of superfluous double quotes that end up in displayed descriptions. (like for i in $(ls -d /*)). Someone just complained earlier that the completer broke, and since reverting this commit fixed it, I spent some time to get file completion after unhandled subcommands and working completion after handled subcommands. I didn't really spend any time looking into why the changed code didn't work, sorry. If you have any theories about it, I'd be happy to try them out though. -- Mikael Magnusson