From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 13276 invoked by alias); 9 Oct 2014 08:32:44 -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: 33396 Received: (qmail 3387 invoked from network); 9 Oct 2014 08:32:30 -0000 X-Spam-Checker-Version: SpamAssassin 3.3.2 (2011-06-06) on f.primenet.com.au X-Spam-Level: X-Spam-Status: No, score=-2.7 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,RCVD_IN_DNSWL_LOW autolearn=ham version=3.3.2 DKIM-Signature: v=1; a=rsa-sha1; c=relaxed/relaxed; d= daniel.shahaf.name; h=x-sasl-enc:date:from:to:cc:subject :message-id:references:mime-version:content-type:in-reply-to; s= mesmtp; bh=gLPeaR8XR0YaWRhyo1+Oz4dY5I0=; b=QJHhzah+zACDYesY+MaB3 uZ1YIf5jfv79NSC58wYf+uijwXWD5cfvOVdrdKPr5Xbdif/5PDz/D5nTo18p2qRp dW2s2gqCHX1BkPA7I0gYysGpdGcRO/8OMCFtGXISEVXvVsffIP9mY+XazzeQWm7I HRu+26ySKtpBkIHjQx8JJw= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed/relaxed; d= messagingengine.com; h=x-sasl-enc:date:from:to:cc:subject :message-id:references:mime-version:content-type:in-reply-to; s= smtpout; bh=gLPeaR8XR0YaWRhyo1+Oz4dY5I0=; b=BzqgGjMA1+Q57WjDQ/Gb M6Aja97WOY/hfi8AkAdR9NSFXdRcQ641wS7smHqyJeqOmEwQLxwmjgBhkeTSEX56 aIIMVkR4FkftF11J0M+HZjYrNg5hpx1wbwD6qMrr6EwWz7X1ExgLtjebEztwKx8Z liWR/MNin31MsR3+bH2RAJk= X-Sasl-enc: SKZztHy/q2NvYYrRGL2J6FxiYCweagrfq9RjI2XQx486 1412842886 Date: Thu, 9 Oct 2014 08:21:24 +0000 From: Daniel Shahaf To: Oliver Kiddle Cc: zsh-workers@zsh.org Subject: Re: [PATCH] Add xxd completion Message-ID: <20141009082124.GC1737@tarsus.local2> References: <20141008082016.GC1712@tarsus.local2> <30554.1412778907@thecus.kiddle.eu> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="Dxnq1zWXvFF0Q93v" Content-Disposition: inline In-Reply-To: <30554.1412778907@thecus.kiddle.eu> User-Agent: Mutt/1.5.21 (2010-09-15) --Dxnq1zWXvFF0Q93v Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Oliver Kiddle wrote on Wed, Oct 08, 2014 at 16:35:07 +0200: > Daniel Shahaf wrote: > > 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. > Thanks for the review. > > +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. > Okay; I removed 'curcontext' and 'ret'. As to 'arguments', I don't see a benefit to inlining it, and keeping it as a separate variable makes the code easier to read. What would be the benefit of inlining it at the single use site? > > +# 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 - > Done. > > +# TODO: xxd - 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 Yes, that's a general issue. For example, instead of: % sudo - -s -- run SHELL -u -- user name I would prefer: % sudo - -s -- run SHELL -u + -- user name or even: % sudo - -s -- run SHELL -u USER -- user name > Incidentally, I ran into another general issue: % xxd - -z -autoskip -a -- a single '*' replaces runs of NUL (toggleable -bits -b -- output in binary digits, rather than hex -cols -c -- specify number of octets per line ... It wasn't immediately obvious to me, but I eventually realized the "-z" in the output was caused by a stray file named "-z" (via the _files completion defined for positional arguments). However, since the filename begins with a hyphen, specifying it as "-z" won't work as intended. Perhaps "xxd -" should offer "./-z" or "-- -z" (two words) as possible completions? > 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. > Okay, I switched the descriptions from the "ARG" convention to the "specify" convention. > The arguments to -c, -g, -l and -s also get in the way of completing > -cols, -groupsize, -len and -seek: > % xxd -grou > 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"' > Done. 'xxd -grou' now DTRTs; however, 'xxd -g' gives me: % xxd -g -groupsize -- specify the number of octets per group and the display doesn't change even if I press again. This seems odd: why am I not offered both -g and -groupsize as possible completions? And if "-groupsize" is offered in the message, why does pressing not complete the command-line word to the one possible completion? > Oliver Thanks for the tips. Revised patch attached. Daniel --Dxnq1zWXvFF0Q93v Content-Type: text/x-patch; charset=us-ascii Content-Disposition: attachment; filename="0001-Add-xxd-completion.patch" >>From 81619a056a812d53f93415d9dddaf8b59aa56eed Mon Sep 17 00:00:00 2001 From: Daniel Shahaf Date: Wed, 8 Oct 2014 17:12:37 +0000 Subject: [PATCH] Add xxd completion --- Completion/Unix/Command/_xxd | 45 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 45 insertions(+) create mode 100644 Completion/Unix/Command/_xxd diff --git a/Completion/Unix/Command/_xxd b/Completion/Unix/Command/_xxd new file mode 100644 index 0000000..12f2c91 --- /dev/null +++ b/Completion/Unix/Command/_xxd @@ -0,0 +1,45 @@ +#compdef xxd + +local arguments + +# Output options compatibility matrix +# +# 0 - options conflict +# 1 - options coexist +# +# (The matrix is symmetric, so implied values are not shown.) +# +# bEipru +# bx10000 +# E-x0001 +# i--x001 +# p---x11 +# r----x0 +# u-----x + +# xxd supports either double or single dashes on long options. +[[ -prefix -- ]] && compset -P - + +arguments=( + # output options + '(-b -bits -i -include -p -postscript -plain -ps -r -reverse -u -uppercase)'{-b,-bits}'[output in binary digits, rather than hex]' + '( -E -EBCDIC -i -include -p -postscript -plain -ps -r -reverse )'{-E,-EBCDIC}'[print human-readable part in EBCDIC rather than ASCII]' + '(-b -bits -E -EBCDIC -i -include -p -postscript -plain -ps -r -reverse )'{-i,-include}'[output in C include file style]' + '(-b -bits -E -EBCDIC -i -include -p -postscript -plain -ps )'{-p,-postscript,-plain,-ps}'[read or write a plain hexdump (no line numbers or ASCII rendering)]' + + '(-b -bits -E -EBCDIC -i -include -r -reverse -u -uppercase)'{-r,-reverse}'[reverse mode\: read a hex dump and output binary data]' + '(-b -bits -r -reverse -u -uppercase)'{-u,-uppercase}'[output upper-case hex digits]' + + {-h,-help}'[display usage message]' + {-v,-version}'[show program version]' + '*'{-a,-autoskip}"[a single '*' replaces runs of NUL (toggleable)]" + + {-c+,-cols}'[specify number of octets per line]: :_guard "[0-9a-fA-Fx]#" "number of octets per line"' + {-g+,-groupsize}'[specify the number of octets per group]: :_guard "[0-9]#" "number of octets per group"' + {-l+,-len}'[specify number of octets to output]: :_guard "[0-9]#" "number of octets to output"' + {-s,-skip,-seek}'[specify file offset to dump from]: :_guard "[0-9]#" "file offset to dump from (absolute or relative)"' + + ':files:_files' +) + +_arguments -S $arguments -- 1.7.10.4 --Dxnq1zWXvFF0Q93v--