Re: [PATCH] Add completion for zathura.

[dana <dana@xxxxxxx>]

On 1 Jul 2018, at 12:11, Doron Behar <doron.behar@xxxxxxxxx> 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@xxxxxxxxx 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

* 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 /)

* Should /usr/lib have precedence over /usr/local/lib? It's not typical

* plugins_dir and pf need made local

On 13 Jun 2018, at 11:31, doron.behar@xxxxxxxxx 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

* 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')

dana