Zsh Mailing List Archive
Messages sorted by: Reverse Date, Date, Thread, Author

Re: PATCH: Fix "35531: fallback on file completion"

On Sat, Mar 19, 2016 at 12:20 AM, Oliver Kiddle <okiddle@xxxxxxxxxxx> 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 <tab>" 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:<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 () {
>> -  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

Messages sorted by: Reverse Date, Date, Thread, Author