[PATCH] Add completion for zathura.

[PATCH] Add completion for zathura.

[doron.behar]

From: Doron Behar <doron.behar@gmail.com>

Make it aware of the plugins installed on the machine.
---
 Completion/Unix/Type/_pspdf   |  2 +-
 Completion/X/Command/_zathura | 45 +++++++++++++++++++++++++++++++++++
 2 files changed, 46 insertions(+), 1 deletion(-)
 create mode 100644 Completion/X/Command/_zathura

diff --git a/Completion/Unix/Type/_pspdf b/Completion/Unix/Type/_pspdf
index 5bb8650b1..1df3f860c 100644
--- a/Completion/Unix/Type/_pspdf
+++ b/Completion/Unix/Type/_pspdf
@@ -1,4 +1,4 @@
-#compdef gsbj gsdj gsdj500 gslj gslp gsnd ps2ascii ghostview mgv pstoedit pstotgif zathura
+#compdef gsbj gsdj gsdj500 gslj gslp gsnd ps2ascii ghostview mgv pstoedit pstotgif
 
 local expl ext
 
diff --git a/Completion/X/Command/_zathura b/Completion/X/Command/_zathura
new file mode 100644
index 000000000..9116d2471
--- /dev/null
+++ b/Completion/X/Command/_zathura
@@ -0,0 +1,45 @@
+#compdef zathura
+
+(( $+functions[_zathura_files] )) ||
+_zathura_files(){
+  local -a plugin_files
+  for plugins_dir in "${opt_args[-p]}" "${opt_args[--plugins-dir]}" "/usr/lib/zathura" "/usr/local/lib/zathura" "/lib/zathura"; do
+    plugin_files=(${plugins_dir}/*.so)
+    if [[ -z "${plugin_files}" ]]; then
+      break
+    fi
+  done
+  if [[ -z "${plugin_files}" ]]; then
+    _files -g "*.pdf(-.)"
+    return
+  fi
+  local -a supported_filetypes
+  for pf in "${plugin_files[@]}"; do
+    if [[ $pf =~ "mupdf" ]]; then
+      supported_filetypes+="pdf"
+      supported_filetypes+="epub"
+      supported_filetypes+="xps"
+    elif [[ $pf =~ "poppler" ]]; then
+      supported_filetypes+="pdf"
+    else
+      supported_filetypes+="${${pf%.so}#${plugins_dir}/lib}"
+    fi
+  done
+  _files -g "*.(${(j.|.)supported_filetypes})(-.)"
+}
+
+_arguments \
+  {-e,--reparent=}'[Reparents to window specified by xid]:xid:_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)' \
+  {-x,--synctex-editor-command=}'[Set the synctex editor command]:command:_cmdstring' \
+  '--synctex-forward=[Jump to the given position]:input: ' \
+  '--synctex-pid=[Instead of looking for an instance having the correct file opened, try only the instance with the given PID]:pid:_pids' \
+  '--fork[Fork into background]' \
+  '(- :)--version[Display version string and exit]' \
+  '(- :)--help[Display help and exit]' \
+  '*:file:_zathura_files'
-- 
2.17.1

Re: [PATCH] Add completion for zathura.

[Doron Behar]

Hey guys, just reminding you there's a patch here ready for your
comments / merge.

On Wed, Jun 13, 2018 at 07:31:52PM +0300, doron.behar@gmail.com wrote:
> From: Doron Behar <doron.behar@gmail.com>
> 
> Make it aware of the plugins installed on the machine.
> ---
>  Completion/Unix/Type/_pspdf   |  2 +-
>  Completion/X/Command/_zathura | 45 +++++++++++++++++++++++++++++++++++
>  2 files changed, 46 insertions(+), 1 deletion(-)
>  create mode 100644 Completion/X/Command/_zathura
> 
> diff --git a/Completion/Unix/Type/_pspdf b/Completion/Unix/Type/_pspdf
> index 5bb8650b1..1df3f860c 100644
> --- a/Completion/Unix/Type/_pspdf
> +++ b/Completion/Unix/Type/_pspdf
> @@ -1,4 +1,4 @@
> -#compdef gsbj gsdj gsdj500 gslj gslp gsnd ps2ascii ghostview mgv pstoedit pstotgif zathura
> +#compdef gsbj gsdj gsdj500 gslj gslp gsnd ps2ascii ghostview mgv pstoedit pstotgif
>  
>  local expl ext
>  
> diff --git a/Completion/X/Command/_zathura b/Completion/X/Command/_zathura
> new file mode 100644
> index 000000000..9116d2471
> --- /dev/null
> +++ b/Completion/X/Command/_zathura
> @@ -0,0 +1,45 @@
> +#compdef zathura
> +
> +(( $+functions[_zathura_files] )) ||
> +_zathura_files(){
> +  local -a plugin_files
> +  for plugins_dir in "${opt_args[-p]}" "${opt_args[--plugins-dir]}" "/usr/lib/zathura" "/usr/local/lib/zathura" "/lib/zathura"; do
> +    plugin_files=(${plugins_dir}/*.so)
> +    if [[ -z "${plugin_files}" ]]; then
> +      break
> +    fi
> +  done
> +  if [[ -z "${plugin_files}" ]]; then
> +    _files -g "*.pdf(-.)"
> +    return
> +  fi
> +  local -a supported_filetypes
> +  for pf in "${plugin_files[@]}"; do
> +    if [[ $pf =~ "mupdf" ]]; then
> +      supported_filetypes+="pdf"
> +      supported_filetypes+="epub"
> +      supported_filetypes+="xps"
> +    elif [[ $pf =~ "poppler" ]]; then
> +      supported_filetypes+="pdf"
> +    else
> +      supported_filetypes+="${${pf%.so}#${plugins_dir}/lib}"
> +    fi
> +  done
> +  _files -g "*.(${(j.|.)supported_filetypes})(-.)"
> +}
> +
> +_arguments \
> +  {-e,--reparent=}'[Reparents to window specified by xid]:xid:_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)' \
> +  {-x,--synctex-editor-command=}'[Set the synctex editor command]:command:_cmdstring' \
> +  '--synctex-forward=[Jump to the given position]:input: ' \
> +  '--synctex-pid=[Instead of looking for an instance having the correct file opened, try only the instance with the given PID]:pid:_pids' \
> +  '--fork[Fork into background]' \
> +  '(- :)--version[Display version string and exit]' \
> +  '(- :)--help[Display help and exit]' \
> +  '*:file:_zathura_files'
> -- 
> 2.17.1
> 

Re: [PATCH] Add completion for zathura.

[dana]

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

* 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@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

* 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

Re: [PATCH] Add completion for zathura.

[Doron Behar]

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 <doron.behar@gmail.com> 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

Re: [PATCH] Add completion for zathura.

[dana]

On 13 Jul 2018, at 07:29, Doron Behar <doron.behar@gmail.com> wrote:
>How quoting `opt_args` and use `(Q)` will be better as for this issue?

The values in opt_args are quoted exactly as they were received from the command
line. So if the user does `zathura -p '/path/to/some/'"file"`, you will receive
it exactly like that, with both sets of quotes, in opt_args. Therefore, if you
don't use (Q), your checks may fail when they would otherwise succeed, since
they're trying to match a path containing literal quotes.

There's probably an even better way to do this, but this is what comes to mind:

  for plugins_dir in \
    ${(Qv)opt_args[(i)-p|--plugins-dir]}(#qN) \
    {/usr/local,/usr,}/lib/zathura(#qN)
  do
    plugins_files=( $plugins_dir/*.so )
    (( $#plugins_files )) && break
  done

On 13 Jul 2018, at 07:29, Doron Behar <doron.behar@gmail.com> wrote:
>These are the options' descriptions I've ended up with:

Looks better to my eye

dana

Re: [PATCH] Add completion for zathura.

[Doron Behar]

On Fri, Jul 13, 2018 at 11:55:09AM -0500, dana wrote:
> On 13 Jul 2018, at 07:29, Doron Behar <doron.behar@gmail.com> wrote:
> >How quoting `opt_args` and use `(Q)` will be better as for this issue?
> 
> The values in opt_args are quoted exactly as they were received from the command
> line. So if the user does `zathura -p '/path/to/some/'"file"`, you will receive
> it exactly like that, with both sets of quotes, in opt_args. Therefore, if you
> don't use (Q), your checks may fail when they would otherwise succeed, since
> they're trying to match a path containing literal quotes.
> 
> There's probably an even better way to do this, but this is what comes to mind:
> 
>   for plugins_dir in \
>     ${(Qv)opt_args[(i)-p|--plugins-dir]}(#qN) \
>     {/usr/local,/usr,}/lib/zathura(#qN)
>   do
>     plugins_files=( $plugins_dir/*.so )
>     (( $#plugins_files )) && break
>   done

Works like a charm. I'll be sending a complete revised patch shortly.

Re: [PATCH] Add completion for zathura.

[Doron Behar]

Hey everyone, just reminding you there's this patch waiting for a merge
or a review / comment.

Thanks.

On Fri, Jul 13, 2018 at 10:52:59PM +0300, doron.behar@gmail.com wrote:
> From: Doron Behar <doron.behar@gmail.com>
> 
> Make it aware of the plugins installed on the machine.
> ---
>  Completion/Unix/Type/_pspdf   |  2 +-
>  Completion/X/Command/_zathura | 48 +++++++++++++++++++++++++++++++++++
>  2 files changed, 49 insertions(+), 1 deletion(-)
>  create mode 100644 Completion/X/Command/_zathura
> 
> diff --git a/Completion/Unix/Type/_pspdf b/Completion/Unix/Type/_pspdf
> index 5bb8650b1..1df3f860c 100644
> --- a/Completion/Unix/Type/_pspdf
> +++ b/Completion/Unix/Type/_pspdf
> @@ -1,4 +1,4 @@
> -#compdef gsbj gsdj gsdj500 gslj gslp gsnd ps2ascii ghostview mgv pstoedit pstotgif zathura
> +#compdef gsbj gsdj gsdj500 gslj gslp gsnd ps2ascii ghostview mgv pstoedit pstotgif
>  
>  local expl ext
>  
> diff --git a/Completion/X/Command/_zathura b/Completion/X/Command/_zathura
> new file mode 100644
> index 000000000..141cadf63
> --- /dev/null
> +++ b/Completion/X/Command/_zathura
> @@ -0,0 +1,48 @@
> +#compdef zathura
> +
> +(( $+functions[_zathura_files] )) ||
> +_zathura_files(){
> +  local -a plugins_files
> +  local plugins_dir
> +  for plugins_dir in \
> +    ${(Qv)opt_args[(i)-p|--plugins-dir]}(#qN) \
> +    {/usr/local,/usr,}/lib/zathura(#qN)
> +  do
> +    plugins_files=( $plugins_dir/*.so )
> +    (( $#plugins_files )) && break
> +  done
> +  if [[ -z "${plugins_files}" ]]; then
> +    _files -g "*.pdf(-.)"
> +    return
> +  fi
> +  local -a supported_filetypes
> +  local pf
> +  for pf in "${plugins_files[@]}"; do
> +    if [[ $pf =~ "mupdf" ]]; then
> +      supported_filetypes+="pdf"
> +      supported_filetypes+="epub"
> +      supported_filetypes+="xps"
> +    elif [[ $pf =~ "poppler" ]]; then
> +      supported_filetypes+="pdf"
> +    else
> +      supported_filetypes+="${${pf%.so}#${plugins_dir}/lib}"
> +    fi
> +  done
> +  _files -g "*.(${(j.|.)supported_filetypes})(-.)"
> +}
> +
> +_arguments -s -S \
> +  {-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]' \
> +  '*:file:_zathura_files'
> -- 
> 2.18.0
> 

[PATCH] Add completion for zathura.

[doron.behar]

From: Doron Behar <doron.behar@gmail.com>

Make it aware of the plugins installed on the machine.
---
 Completion/Unix/Type/_pspdf   |  2 +-
 Completion/X/Command/_zathura | 48 +++++++++++++++++++++++++++++++++++
 2 files changed, 49 insertions(+), 1 deletion(-)
 create mode 100644 Completion/X/Command/_zathura

diff --git a/Completion/Unix/Type/_pspdf b/Completion/Unix/Type/_pspdf
index 5bb8650b1..1df3f860c 100644
--- a/Completion/Unix/Type/_pspdf
+++ b/Completion/Unix/Type/_pspdf
@@ -1,4 +1,4 @@
-#compdef gsbj gsdj gsdj500 gslj gslp gsnd ps2ascii ghostview mgv pstoedit pstotgif zathura
+#compdef gsbj gsdj gsdj500 gslj gslp gsnd ps2ascii ghostview mgv pstoedit pstotgif
 
 local expl ext
 
diff --git a/Completion/X/Command/_zathura b/Completion/X/Command/_zathura
new file mode 100644
index 000000000..141cadf63
--- /dev/null
+++ b/Completion/X/Command/_zathura
@@ -0,0 +1,48 @@
+#compdef zathura
+
+(( $+functions[_zathura_files] )) ||
+_zathura_files(){
+  local -a plugins_files
+  local plugins_dir
+  for plugins_dir in \
+    ${(Qv)opt_args[(i)-p|--plugins-dir]}(#qN) \
+    {/usr/local,/usr,}/lib/zathura(#qN)
+  do
+    plugins_files=( $plugins_dir/*.so )
+    (( $#plugins_files )) && break
+  done
+  if [[ -z "${plugins_files}" ]]; then
+    _files -g "*.pdf(-.)"
+    return
+  fi
+  local -a supported_filetypes
+  local pf
+  for pf in "${plugins_files[@]}"; do
+    if [[ $pf =~ "mupdf" ]]; then
+      supported_filetypes+="pdf"
+      supported_filetypes+="epub"
+      supported_filetypes+="xps"
+    elif [[ $pf =~ "poppler" ]]; then
+      supported_filetypes+="pdf"
+    else
+      supported_filetypes+="${${pf%.so}#${plugins_dir}/lib}"
+    fi
+  done
+  _files -g "*.(${(j.|.)supported_filetypes})(-.)"
+}
+
+_arguments -s -S \
+  {-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]' \
+  '*:file:_zathura_files'
-- 
2.18.0