Re: [PATCH] Add xxd completion

[Oliver Kiddle <okiddle@xxxxxxxxxxx>]

Daniel Shahaf wrote:
> Attached completion for xxd (a hex dump utility from Vim).  It works,

Great, thanks.

> but I haven't written completion functions before so I might have
> overlooked something.

With that in mind, I've taken a look and have a couple of
recommendations.

> +local curcontext="$curcontext" ret=1 arguments

None of these are actually needed. curcontext is only relevant if you
call _arguments or _values with the -C option. You do use ret but a zsh
function will pass on the return status of the last command it calls and
_xxd only adds matches with a final call to _arguments. Finally the
arguments array is only expanded once and could be specified inline at
that point.

> +# TODO: permit two hyphens (--autoskip, --groupsize, etc)

Given that two dashes is accepted but not documented by xxd and that it
also works for the short options, e.g. --u, I would just strip a dash as
follows:

  [[ -prefix -- ]] && compset -P -

> +# TODO: xxd -<tab> should show '-x' and '-x:' differently - give visual hint that there's a required argument

Unless I'm missing something, there's nothing xxd specific in that
desire. Perhaps it should be considered for the general case. In many
cases, you can tell options that take an argument by the fact that the
description starts with the word "specify". Taking -c as an example, you
have:

> +    {-c+,-cols}'[output ARG octets per line]:number of octets per line'

Until I checked, and found otherwise, I was guessing that the "ARG" came
from a direct cut and paste from the -help output.

The arguments to -c, -g, -l and -s also get in the way of completing
-cols, -groupsize, -len and -seek:
  % xxd -grou<tab>
  number of octets per group
  option
  -groupsize

To _arguments, the rou characters could be the number of octets. By
using _guard, we can tell it that the number can only be the characters
0-9 (xxd allows hex so a few more characters are allowed there). With
_guard, it looks like this:

  {-c+,-cols}'[specify number of octets per line]: :_guard "[0-9a-fA-Fx]#" "number of octets per line"'

Oliver