zsh-workers
 help / color / mirror / code / Atom feed
From: Oliver Kiddle <okiddle@yahoo.co.uk>
To: zsh-workers@zsh.org
Subject: Re: PATCH: Fix "35531: fallback on file completion"
Date: Sat, 19 Mar 2016 00:20:47 +0100	[thread overview]
Message-ID: <31107.1458343247@thecus.kiddle.eu> (raw)
In-Reply-To: <1458317336-27434-1-git-send-email-mikachu@gmail.com>

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?

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:<function>:<completer>:<command>:<arg>:<tag>

We typically put subcommands into <command> by changing it to
<command>-<subcommand>. If you use a colon instead of a dash, the
subcommand will be in <arg> and thereafter, everything gets messed up.

> -(( $+functions[_adb_check_log_redirect] )) ||
> +(( $+functions[adb_check_log_redirect] )) ||
>  _adb_check_log_redirect () {

Are you sure you want to back that out?
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.

> -  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.

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.

Oliver


  parent reply	other threads:[~2016-03-18 23:27 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-03-18 16:08 Mikael Magnusson
2016-03-18 16:11 ` Mikael Magnusson
2016-03-18 23:20 ` Oliver Kiddle [this message]
2016-03-19  1:13   ` Mikael Magnusson
2016-03-19  5:56     ` Oliver Kiddle
2016-03-19 12:36       ` PATCH: _adb: fix remote file completion + various fixes Mikael Magnusson

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=31107.1458343247@thecus.kiddle.eu \
    --to=okiddle@yahoo.co.uk \
    --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).