From: Oliver Kiddle <opk@zsh.org>
To: Marc Chantreux <eiro@phear.org>
Cc: Zsh hackers list <zsh-workers@zsh.org>
Subject: Re: 2 new patches for _surfraw
Date: Fri, 09 Apr 2021 23:53:07 +0200 [thread overview]
Message-ID: <84376-1618005187.938917@aiYa.Xwq7.uanX> (raw)
In-Reply-To: <YHCv4PKK/2ah9s32@prometheus.u-strasbg.fr>
Marc Chantreux wrote:
> any review is warmly welcome.
This mostly looks all good so there's not much I can say.
> - '*:string:_guard "^-*" "search string"'
> + '*:strings:_guard "^-*" "search string"'
Thats not actually a tag there but a description. _arguments uses tags
and groups with names like argument-rest and option-J-1. _guard doesn't
take a tag as a parameter and it replaces any description with the new
one. _message may pick it up the tag from $curtag. I know this is far
from intuitive but in this case, it is fine to just do:
'*: :_guard "^-*" "search string"'
> - local -UT XDG_CONFIG_DIRS xcd
This looks like a duplicate of the patch in users/26579 from here on.
That was already applied in ccc7ff9. I don't need a fresh patch without
this unless you're sending one anyway.
Looking at the existing _surfraw completion there are other things that
might be improved on or that I suspect may be broken:
| _arguments -C -A \
| '-browser=[set browser]:browser:_command_names' \
The -A option to _arguments takes an argument. "-*" is common to match
options, indicating that no more options are allowed after the first
non-option argument. In this case, it is preventing -browser from being
completed. It is also worth verifying if -S would be applicable, i.e. --
terminates options and -s, i.e. -tq is allowed as a short form of -t -q.
Aside from that, it should probably be _command_names -e to limit it the
external commands.
| '-quiet:bool:(yes no)' \
Should this be -quiet= ?
Commands tend to be consistent and = was needed for -browser and
-escape-url-args. For many of the elvi, it is =- which means you have to
use, e.g. -type=soft and not -type soft
| '(-t -text)'{-t,-text}'[back to the yellow brick road]'
Are -t and -g mutually exlusive? If so, they could be listed in each
others exclusion list, e.g. '(-t -text -g -graphical)'{....
There may be other possible exclusions, e.g. what options are possible
after -elvi?
| cite)
| _arguments \
| '-results=-:[number of results to return]' \
This doesn't look right. The colon should probably come after the
square brackets. And as the description in square brackets is a
description of the -results option and not of the argument that follows
it, I'd probably add a verb like "specify" or "give" to the beginning.
This occurs more than once in the file.
| '-lines=-[specify maximum lines per message]:lines:(0 5 10 50 100)' \
Perhaps use compadd -o nosort. If any number is allowed it may be better
to leave it blank instead of providing matches.
| '-gg=-[search [Google Groups]:enable:(yes no)' \
There's an extra opening bracket in this.
Oliver
next prev parent reply other threads:[~2021-04-09 21:53 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-04-09 19:49 Marc Chantreux
2021-04-09 21:53 ` Oliver Kiddle [this message]
2021-04-09 23:10 ` Daniel Shahaf
2021-04-10 0:05 ` Oliver Kiddle
2021-04-10 8:38 ` Marc Chantreux
2021-04-11 20:23 ` Oliver Kiddle
2021-04-12 8:23 ` Marc Chantreux
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=84376-1618005187.938917@aiYa.Xwq7.uanX \
--to=opk@zsh.org \
--cc=eiro@phear.org \
--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).