Re: [PATCH 1/1] Add completion for zathura.

[Oliver Kiddle <okiddle@xxxxxxxxxxx>]

doron.behar@xxxxxxxxx wrote:

Thanks for this, I have some comments:

> +(( $+functions[_zathura_files] )) ||
> +_zathura_files(){
> +  for plugins_dir in "${opt_args[-p]}" "${opt_args[--plugins-dir]}" "/usr/lib/zathura"; do

For my installation of zathura, this finds no plugins. Perhaps they are
compiled in. It handles at least PDF and PostScript.

> +  local files_regex="*.{${supported_filetypes[1]},"
> +  for (( i = 2 ; i < ${#supported_filetypes[*]}; i ++)); do
> +    files_regex="${files_regex}""${supported_filetypes[$i]}"","
> +  done
> +  files_regex="${files_regex}""${supported_filetypes[-1]}""}"
> +  _files -g "${files_regex}"

I think this can be simplified to avoid the loop when constructing the
file glob:
  "*.(${(j.|.)supported_filetypes})(-.)"

> +
> +_arguments \
> +  {-e,--reparent=}'[Reparents to window specified by xid]:XID: ' \

As with your previous function, please stick to conventions on case for
descriptions: don't use uppercase for the first word. And don't put the
headings (like XID, PATH, NUMBER, PASSWORD) in block capitals.

X IDs are completed by _x_window.


> +  {-c,--config-dir=}'[Path to the config directory]:PATH:{_files -/}' \
> +  {-d,--data-dir=}'[Path to the data directory]:PATH:{_files -/}' \
> +  {-p,--plugins-dir=}'[Path to the directory containing plugins]:PATH:{_files -/}' \
> +  {-w,--password=}"[The document's password]:PASSWORD: " \
> +  {-P,--page=}'[Opens the document at the given page number]:NUMBER: ' \
> +  {-l,--log-level=}'[Set log level]:LEVEL:(debug, info, warning, error)' \

Remove the commas in the list. It is a space separated list when you
specify multiple options like that.

> +  {-x,--synctex-editor-command=}'[Set the synctex editor command]:COMMAND:_command' \

_command is the completer for the command reserved word so is not
applicable here. You probably want either _command_names -e or
_cmdstring.

Oliver