From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 4494 invoked by alias); 14 Jul 2018 09:21:23 -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: List-Unsubscribe: X-Seq: 43167 Received: (qmail 7991 invoked by uid 1010); 14 Jul 2018 09:21:23 -0000 X-Qmail-Scanner-Diagnostics: from mail-wr1-f54.google.com by f.primenet.com.au (envelope-from , uid 7791) with qmail-scanner-2.11 (clamdscan: 0.99.2/21882. spamassassin: 3.4.1. Clear:RC:0(209.85.221.54):SA:0(-1.9/5.0):. Processed in 1.221228 secs); 14 Jul 2018 09:21:23 -0000 X-Spam-Checker-Version: SpamAssassin 3.4.1 (2015-04-28) on f.primenet.com.au X-Spam-Level: X-Spam-Status: No, score=-1.9 required=5.0 tests=BAYES_00,FREEMAIL_FROM, RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,SPF_PASS,T_DKIM_INVALID autolearn=ham autolearn_force=no version=3.4.1 X-Envelope-From: doron.behar@gmail.com X-Qmail-Scanner-Mime-Attachments: | X-Qmail-Scanner-Zip-Files: | DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=date:from:to:subject:message-id:mail-followup-to:references :mime-version:content-disposition:in-reply-to:user-agent; bh=uNeOwv5c5IaZ7DV3YXqDZK41cplOTo991mzfn5B6NpM=; b=mzss0yFruzZxhKlrXs/7j0vAUC4I8wTe+4AbiIaDkwvTRZLobQsJnwBgy+hcJtbR3s jGBg034/spnsfz/cYLZbexPByVsYD99PpmhXV9X/h3yJrrkE8On2J5MgcNfxYcLxoZjQ Eu39MOElvc1LNGMlqqDVd0zleWXazHG6/boT5hSAk0ALX/YKgQc7XoollRo0WadLUwq8 Xskx93TpjtaMJBh7XpHoqZS456t+9vX0wGUO4wKYz6M7gMwA/kP+3Tn3ALFjLQLH+gsH ovrz3MaN5b2gd+Lnt9Ebo598D6lz5HBafXUuR/b6UxxxCfipU1rgqB7tSRffaKiPD+Uz lSnw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:subject:message-id:mail-followup-to :references:mime-version:content-disposition:in-reply-to:user-agent; bh=uNeOwv5c5IaZ7DV3YXqDZK41cplOTo991mzfn5B6NpM=; b=A8N5iwzRFAbXokNM3J5c3u3218vDjcQnPfpqYd0ZS5MA+o9lTRGmxVXV0F7VRFMhOt TZ/EvFbSnnFtElXpeb33IhCaOjktrDi8wbUH0GZs2pFqfKhEHnFA9V95D9HXVrqWN5Hv 2ND7XiULoWNH21rd8m49vkpWGVxduzfiJpkU1n8fMpz5RKqu6AR4zt4r7z2qfTurHxoi Dt6n1YGU3zwx8datTjYlN78DFPb8M4XTjWK5bMgHj2tVdjnP6VtbCC32LRAyDOMpFC7X pYBjeE1oyDU9qVRYGcFrKklNX1FqhVGopUOwLBUO9k+vBYJHJU5UWTfM6q4yUFLiZ5wC 9Owg== X-Gm-Message-State: AOUpUlFTUbZgGFxiucciNFPTWJBGAUlYY85G7ZQUYyJI7owMnoP2+v+c wu3+KK4vg8055Bnrey9wqusE+2R7 X-Google-Smtp-Source: AAOMgpdJUnfnGGpVcUqMLUe1i8BlSIGca2knDIZA+qprcX5utGVr/AV7EdUmpXvHdq1y61m1tim2+A== X-Received: by 2002:a5d:4210:: with SMTP id n16-v6mr4560211wrq.55.1531485031053; Fri, 13 Jul 2018 05:30:31 -0700 (PDT) Date: Fri, 13 Jul 2018 15:29:09 +0300 From: Doron Behar To: zsh-workers@zsh.org, dana@dana.is Subject: Re: [PATCH] Add completion for zathura. Message-ID: <20180713122909.o5rpg6ocoigx6sv3@NUC.doronbehar.com> Mail-Followup-To: zsh-workers@zsh.org, dana@dana.is References: <20180613163152.28843-1-doron.behar@gmail.com> <20180701171158.6uzs27ooafm4pk42@NUC.doronbehar.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: User-Agent: NeoMutt/20180622 Sorry for the long wait on my reply. I've responded to your comments below. I'll resend a patch when this is done. On Sun, Jul 01, 2018 at 02:06:57PM -0500, dana wrote: > On 1 Jul 2018, at 12:11, Doron Behar wrote: > >Hey guys, just reminding you there's a patch here ready for your > >comments / merge. > > Some things i noticed (disregard anything that's too nit-picky i guess): > > On 13 Jun 2018, at 11:31, doron.behar@gmail.com wrote: > >+_zathura_files(){ > > * I don't think you want to quote the variables in your for loop; whenever > they're empty, you'll be looking for files matching /*.so > > * However, you maybe *do* want to apply (Q) to the opt_args values, since > they're taken 'raw' from the command line, and quoting file paths is a very > common thing that people do How quoting `opt_args` and use `(Q)` will be better as for this issue? > > * Is the first check really supposed to be [[ -z $plugin_files ]] rather than > -n? It seems like it's going to immediately break out of the loop unless > _arguments found -p (or there are random shared libs in /) You were right, the opposite is correct - the break is needed when have reached a plugins directory which has no shared objects in it. But I don't see a difference between `-z` and `-n` so I'll show here what I've come up with so far: for plugins_dir in "${opt_args[-p]}" "${opt_args[--plugins-dir]}" "/usr/local/lib/zathura" "/usr/lib/zathura" "/lib/zathura"; do if [[ ! -z "${plugins_dir}" ]]; then plugins_files=("${plugins_dir}"/*.so) if [[ ! -z "${plugins_files}" ]]; then break fi fi done > > * Should /usr/lib have precedence over /usr/local/lib? It's not typical > Agree. > * plugins_dir and pf need made local Done. > > On 13 Jun 2018, at 11:31, doron.behar@gmail.com wrote: > >+_arguments \ > > * I think this could use -s and -S > > * Since Zathura uses GLib's dumb option parser, i think (?) it's right that > these are given in -o,--opt= form rather than the usual -o+,--opt=. Might want > to make a note tho; i always double-take when i see that personally Done. > > * The descriptions for the options are inconsistently worded (some are verb > phrases, some use the indicative mood, some are just describing the argument) > and they don't conform to the usual capitalisation conventions > > * Some of the optarg descriptions aren't that helpful ('number') > These are the options' descriptions I've ended up with: {-e,--reparent=}'[specify xid of window to reparent to]:xid:_x_window' \ {-c,--config-dir=}'[specify path to the config directory]:config directory:{_files -/}' \ {-d,--data-dir=}'[specify path to the data directory]:data directory:{_files -/}' \ {-p,--plugins-dir=}'[specify path to the directory containing plugins]:plugins directory:{_files -/}' \ {-w,--password=}"[specify a password for the document]:password: " \ {-P,--page=}'[open the document at the given page number]:page number: ' \ {-l,--log-level=}'[set log level]:log level:(debug info warning error)' \ {-x,--synctex-editor-command=}'[specify synctex editor command]:synctex editor command:_cmdstring' \ '--synctex-forward=[jump to the given position]:synctex position: ' \ '--synctex-pid=[specify pid of an instance having the correct file opened]:synctex pid:_pids' \ '--fork[fork into background]' \ '(- :)--version[display version string and exit]' \ '(- :)--help[display help and exit]' \ > dana > Doron