[PATCH] Completion: _opkg: Support gain-privileges

[PATCH] Completion: _opkg: Support gain-privileges

[dana]

This is apparently necessary even for 'read-only' tasks on systems where you
don't already have root (like Synology).

Also, it's a bit annoying if you have caching enabled and it caches an empty
array because you forgot to use sudo (or various other reasons depending on
the function). Maybe _store_cache should learn an option to make it ignore
empty parameters

dana


diff --git a/Completion/Linux/Command/_opkg b/Completion/Linux/Command/_opkg
index d64567681..ddaefe1eb 100644
--- a/Completion/Linux/Command/_opkg
+++ b/Completion/Linux/Command/_opkg
@@ -35,6 +35,10 @@
 # % zstyle ':completion:*:opkg:*' conf-paths <pattern> ...
 #   Set to one or more paths or glob patterns to override the defaults used when
 #   searching opkg configuration data.
+#
+# Elevated privileges may be necessary to complete package names, etc.; consider
+# setting the gain-privileges style as follows:
+# zstyle ':completion:*:(ipkg|opkg)/*' gain-privileges yes
 
 ##
 # Check cache validity.
@@ -105,7 +109,7 @@ _opkg_arch_prio() {
   }
 
   # Already configured arches
-  tmp=( ${(f)"$( _call_program architectures $svc print-architecture )"} )
+  tmp=( ${(f)"$( _call_program -p architectures $svc print-architecture )"} )
   tmp=( ${${tmp##arch[ ]##}%% *} )
 
   tmp+=(
@@ -182,7 +186,9 @@ _opkg_pkg_all() {
 
   { (( ! $#_opkg_cache_pkg_all )) || _cache_invalid opkg-pkg-all } &&
   ! _retrieve_cache opkg-pkg-all && {
-    _opkg_cache_pkg_all=( ${(f)"$( _call_program pkg-all ${svc:-opkg} list )"} )
+    _opkg_cache_pkg_all=( ${(f)"$(
+      _call_program -p pkg-all ${svc:-opkg} list )"}
+    )
     _opkg_cache_pkg_all=( ${(@)_opkg_cache_pkg_all##[[:space:]]*} )
     _opkg_cache_pkg_all=( ${(@)_opkg_cache_pkg_all%%[[:space:]]*} )
     _store_cache opkg-pkg-all _opkg_cache_pkg_all
@@ -207,7 +213,7 @@ _opkg_pkg_inst() {
   { (( ! $#_opkg_cache_pkg_inst )) || _cache_invalid opkg-pkg-inst } &&
   ! _retrieve_cache opkg-pkg-inst && {
     _opkg_cache_pkg_inst=( ${(f)"$(
-      _call_program pkg-inst ${svc:-opkg} list-installed
+      _call_program -p pkg-inst ${svc:-opkg} list-installed
     )"} )
     _opkg_cache_pkg_inst=( ${(@)_opkg_cache_pkg_inst##[[:space:]]*} )
     _opkg_cache_pkg_inst=( ${(@)_opkg_cache_pkg_inst%%[[:space:]]*} )
@@ -257,7 +263,7 @@ _opkg_pkg_upgr() {
   { (( ! $#_opkg_cache_pkg_upgr )) || _cache_invalid opkg-pkg-upgr } &&
   ! _retrieve_cache opkg-pkg-upgr && {
     _opkg_cache_pkg_upgr=( ${(f)"$(
-      _call_program pkg-upgr ${svc:-opkg} list-upgradable
+      _call_program -p pkg-upgr ${svc:-opkg} list-upgradable
     )"} )
     _opkg_cache_pkg_upgr=( ${(@)_opkg_cache_pkg_upgr##[[:space:]]*} )
     _opkg_cache_pkg_upgr=( ${(@)_opkg_cache_pkg_upgr%%[[:space:]]*} )

Re: [PATCH] Completion: _opkg: Support gain-privileges

[Daniel Shahaf]

dana wrote on Fri, Jan 11, 2019 at 17:34:17 -0600:
> Also, it's a bit annoying if you have caching enabled and it caches an empty
> array because you forgot to use sudo (or various other reasons depending on
> the function). Maybe _store_cache should learn an option to make it ignore
> empty parameters

I don't think so.  Even an "empty" result could be expensive to compute:
for example, it might require a network round-trip.  I think _opkg
shouldn't call _store_cache when _call_program returns non-zero.

Cheers,

Daniel

Re: [PATCH] Completion: _opkg: Support gain-privileges

[dana]

On 12 Jan 2019, at 11:10, Daniel Shahaf <d.s@daniel.shahaf.name> wrote:
>I don't think so.  Even an "empty" result could be expensive to compute:
>for example, it might require a network round-trip.  I think _opkg
>shouldn't call _store_cache when _call_program returns non-zero.

Well, i think that's effectively the same thing in this case and certain
others i'm imagining. (Or at least i assume so; there's not really any
documentation for opkg that i'm aware of, so who knows what actually causes it
to return non-zero.) Just felt like it might save some boiler-plate, which the
caching logic is extremely heavy in already, to have a standard option handle
it. I can just add the guard to each _store_cache call myself, though, obv

dana

Re: [PATCH] Completion: _opkg: Support gain-privileges

[Daniel Shahaf]

dana wrote on Sat, Jan 12, 2019 at 13:08:30 -0600:
> On 12 Jan 2019, at 11:10, Daniel Shahaf <d.s@daniel.shahaf.name> wrote:
> >I don't think so.  Even an "empty" result could be expensive to compute:
> >for example, it might require a network round-trip.  I think _opkg
> >shouldn't call _store_cache when _call_program returns non-zero.
> 
> Well, i think that's effectively the same thing in this case and certain
> others i'm imagining. (Or at least i assume so; there's not really any
> documentation for opkg that i'm aware of, so who knows what actually causes it
> to return non-zero.) Just felt like it might save some boiler-plate, which the
> caching logic is extremely heavy in already, to have a standard option handle
> it. I can just add the guard to each _store_cache call myself, though, obv

Sorry, I thought you meant making that the default behaviour.  My bad.

I don't think expecting callers to check the exit code of commands they
call is an unusual or onerous requirement, not that it adds much
boilerplate:

diff --git a/Completion/Linux/Command/_opkg b/Completion/Linux/Command/_opkg
index d64567681..deeb766d9 100644
--- a/Completion/Linux/Command/_opkg
+++ b/Completion/Linux/Command/_opkg
@@ -181,12 +181,11 @@ _opkg_pkg_all() {
   copts=( "$@" )
 
   { (( ! $#_opkg_cache_pkg_all )) || _cache_invalid opkg-pkg-all } &&
-  ! _retrieve_cache opkg-pkg-all && {
-    _opkg_cache_pkg_all=( ${(f)"$( _call_program pkg-all ${svc:-opkg} list )"} )
+  _retrieve_cache opkg-pkg-all || if _opkg_cache_pkg_all=( ${(f)"$( _call_program pkg-all ${svc:-opkg} list )"} ); then
     _opkg_cache_pkg_all=( ${(@)_opkg_cache_pkg_all##[[:space:]]*} )
     _opkg_cache_pkg_all=( ${(@)_opkg_cache_pkg_all%%[[:space:]]*} )
     _store_cache opkg-pkg-all _opkg_cache_pkg_all
-  }
+  fi
   (( $#upd )) && return 0
 
   (( $#_opkg_cache_pkg_all )) || {

But all that said, if the proposed new --option would be useful to many
callers, then of course we could/should add it.

Cheers,

Daniel