From mboxrd@z Thu Jan 1 00:00:00 1970 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on inbox.vuxu.org X-Spam-Level: X-Spam-Status: No, score=-3.4 required=5.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,MAILING_LIST_MULTI,RCVD_IN_DNSWL_MED,UNPARSEABLE_RELAY autolearn=ham autolearn_force=no version=3.4.4 Received: (qmail 22496 invoked from network); 9 Apr 2021 21:53:45 -0000 Received: from zero.zsh.org (2a02:898:31:0:48:4558:7a:7368) by inbox.vuxu.org with ESMTPUTF8; 9 Apr 2021 21:53:45 -0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=zsh.org; s=rsa-20200801; h=List-Archive:List-Owner:List-Post:List-Unsubscribe: List-Subscribe:List-Help:List-Id:Sender:Message-ID:Date:Content-ID: Content-Type:MIME-Version:Subject:To:References:From:In-reply-to:cc:Reply-To: Content-Transfer-Encoding:Content-Description:Resent-Date:Resent-From: Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID; bh=wD3lv+cgoZj7sdH5Uru6sDIaSTt7KWdFaBPR/rUMnkk=; b=WNSFeYymW4v0t9YZz4+vDtwIto K4B1NYs3P2oMW55LjVkeFNiJHpYq0OXsTxP4tznCuAmj13R66VN4aM2pd99S3Gfy359XAAYt0LoIk rdFfdTswUdlKIiePRwGAdZa+K8fvhQQledQL11t0TI21MT6rsGhPmVVej64F5YKblLGdoNjI04Lr3 9ez2J0zDDL10T6O8N+9DWSQqbCseMZW0vfC5LgbpSy36BIb6U6/ydydWA+cc2zksSIDn8t26+9nzB q6R1ptEuFzkKn1nkodfqWCFkI/UWFJhkad8794lMmcVwKwidVrGYD8erYFuuVqv83QEprKvAOJ8qW Drt+pNlw==; Received: from authenticated user by zero.zsh.org with local id 1lUz4S-000DtH-5v; Fri, 09 Apr 2021 21:53:44 +0000 Received: from authenticated user by zero.zsh.org with esmtpsa (TLS1.3:TLS_AES_256_GCM_SHA384:256) id 1lUz3t-000DdP-3c; Fri, 09 Apr 2021 21:53:09 +0000 Received: from [192.168.178.21] (helo=hydra) by mail.kiddle.eu with esmtp(Exim 4.93.0.4) (envelope-from ) id 1lUz3r-000Lwv-UO; Fri, 09 Apr 2021 23:53:08 +0200 cc: Zsh hackers list In-reply-to: From: Oliver Kiddle References: To: Marc Chantreux Subject: Re: 2 new patches for _surfraw MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-ID: <84375.1618005187.1@hydra> Date: Fri, 09 Apr 2021 23:53:07 +0200 Message-ID: <84376-1618005187.938917@aiYa.Xwq7.uanX> X-Seq: 48449 Archived-At: X-Loop: zsh-workers@zsh.org Errors-To: zsh-workers-owner@zsh.org Precedence: list Precedence: bulk Sender: zsh-workers-request@zsh.org X-no-archive: yes List-Id: List-Help: List-Subscribe: List-Unsubscribe: List-Post: List-Owner: List-Archive: 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