From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 21220 invoked by alias); 19 Mar 2016 06:03:39 -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: 38185 Received: (qmail 25205 invoked from network); 19 Mar 2016 06:03:36 -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=yahoo.co.uk; s=s2048; t=1458366975; bh=915Y4nJJq/k0Gpw/qYTNdzl13Qd4fYNYS02rGHhuNew=; h=In-reply-to:From:References:To:Subject:Date:From:Subject; b=hGyqMejVINJZ2+fDI9Xtis1glT+MmSUkPe0UpqIHH+jNU1qeRyCxwIQAobmh9eLCEVdnc9Ip+niYu8oewaqShdkwUw+Eu91jdY3qWQqDDgdaTjCdSlhTcyAqT5Z1ZoeagbK+mtlQkQfSQtck+doQ7wQBJO5xkLFwga6uLGY5hnMVIvYJF1gMKtvbUFERs17J3hM3p05mTxrDY6pIuHgijIAELnQKKHXjlw+v0fC35gMmFyKj2UDDxDOCypJprW952ZDUh0Qm5JvVzbz88oqoRc0hRAN74HZ4j4J1hCdqMLGnRihTk7u0+Gen8pAa+06qgwjrzlbyQp2JG1tXXAbUgQ== X-Yahoo-Newman-Id: 893252.15875.bm@smtp110.mail.ir2.yahoo.com X-Yahoo-Newman-Property: ymail-3 X-YMail-OSG: crPeHZwVM1lfea0ZKQnO4jBMWN5UiMwHCE7oRZwRgk7LcdG ak.ogyfv27aoRS.cousrdx6CsGJ7OV0K.6fxttrPf3UCgBOoNkxKYoFTGjip 9Le9U38Pp_7ApVEl64_9PnzxXBX_zhZv7FSrQZ6oUvxXeCV8bnMEjgEYF1M2 kDZNXrXFZpGd22EEirfA4ZE.iwRu2zPqKD3HzOZDIy9UMloOHvGxJHXPdn2K hJcsDLFMJ.QeFENudh_kXnStudE.Ylj6sJv0kHEVjmYrm1tzx_bBQT7D_jil VTiQfpJq0490lLL9KvU6Xq0X0VvNdu.B1Ss541aPiiVG2FAvADtnvl_ZpL0e ZBnX_2BE04bSILwd8lew6w52xD.bSLS8DlXBOuPnXu6MXV9RTfReVxioDNWc m.Nes.WiLNnlTINsjX4ItmpPif9xk2wzp3UE0oqOz8FHmkuiXghuUTLY7q1t v8CoVq6jigQKoVYw_rtBcM4YkgDfQRv.aZbdkDC2Jff4xABRaMHSTv6eaYFH RBiU3pqp_XmGDDq_GoFiA0L0Pt.6j.w-- X-Yahoo-SMTP: opAkk_CswBAce_kJ3nIPlH80cJI- In-reply-to: From: Oliver Kiddle References: <1458317336-27434-1-git-send-email-mikachu@gmail.com> <31107.1458343247@thecus.kiddle.eu> To: zsh workers Subject: Re: PATCH: Fix "35531: fallback on file completion" MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-ID: <32471.1458366801.1@thecus.kiddle.eu> Date: Sat, 19 Mar 2016 06:56:14 +0100 Message-ID: <32501.1458366974@thecus.kiddle.eu> Mikael Magnusson 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. I think the problem was a missing trailing colon when matching $curcontext. The following patch is against the original before your commit. Given that you squashed the backout and new changes together, would you mind merging this. Again, my only testing first involves hacking it to continue when it doesn't find adb working. > >> 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 _default is nearly always the best option with unrecognised subcommands. In the past, we've had various approaches such as _message which might be helpful if a user has mistyped a sub-command but _default is consistent with how we handle top-level commands for which there is no completion and is the most graceful fallback when new subcommands have been added since the function was written. It'd also be more graceful in the case of adb on Solaris, though we should probably just stuff an if condition of [[ $OSTYPE = solaris* ]] in the top. > 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 Yuk. We could do better when it comes to code-reviewing new functions while the original author is still around. Complaining about everything before we get past the first dozen lines isn't really going to be encouraging, however. In this case, the local variables should be moved inside adb() otherwise they are only local the first time. setopt nonomatch should not be required, though it is hard to know why it is there. Then we have a variable naming scheme that leaves me wondering if stuff like ADB_DEVICE_SPECIFICATION is an environmenent variable with a meaning to adb. The -l option should not be there: autoload +X _adb is close if you want to see the result of autoloading. And it goes on... But as I've apparently demonstrated, I'm not able to clean it up without also accidentally breaking it in the process. Oliver diff --git a/Completion/Unix/Command/_adb b/Completion/Unix/Command/_adb index 88aca24..e6f7710 100644 --- a/Completion/Unix/Command/_adb +++ b/Completion/Unix/Command/_adb @@ -100,25 +100,25 @@ _adb_dispatch_command () { fi case ${curcontext} in - (*:adb-shell) + (*:adb-shell:) (( $+functions[_adb_dispatch_shell] )) && _adb_dispatch_shell ;; - (*:adb-connect|*:adb-disconnect) + (*:adb-connect:|*:adb-disconnect:) (( $+functions[_adb_dispatch_connection_handling] )) && _adb_dispatch_connection_handling ;; - (*:adb-logcat) + (*:adb-logcat:) (( $+functions[_adb_dispatch_logcat] )) && _adb_dispatch_logcat ;; - (*:adb-push) + (*:adb-push:) (( $+functions[_adb_dispatch_push] )) && _adb_dispatch_push ;; - (*:adb-pull) + (*:adb-pull:) (( $+functions[_adb_dispatch_pull] )) && _adb_dispatch_pull ;; - (*:adb-install) + (*:adb-install:) (( $+functions[_adb_dispatch_install] )) && _adb_dispatch_install ;; - (*:adb-uninstall) + (*:adb-uninstall:) (( $+functions[_adb_dispatch_uninstall] )) && _adb_dispatch_uninstall ;; (*:adb-*)