Re: man completion

[Bart Schaefer <schaefer@xxxxxxxxxxxxxxxx>]

On Sun, May 15, 2022 at 9:45 AM Bart Schaefer <schaefer@xxxxxxxxxxxxxxxx> wrote:
>
> The completion function is calling $(manpath 2>/dev/null), but it's
> caching the result, so if you change $MANPATH that's not being picked
> up.
>
> It also clobbers the cache if called with the -M or -m options and
> doesn't reset it, which is clearly wrong.

Here's a patch that attempts to (1) skip the cache entirely if the -M
option appears and (2) assure a cache miss if $MANPATH has changed.

The question is whether it's worth this much effort to avoid running
$(manpath 2>/dev/null) every time.

Also ... I didn't change this part, but doing ${(s.:.)...} twice seems
redundant or even potentially wrong?

+      mp=( ${(s.:.)$(manpath 2>/dev/null)} )
+      [[ "$mp" == *:* ]] && mp=( ${(s.:.)mp} )

diff --git a/Completion/Unix/Command/_man b/Completion/Unix/Command/_man
index dba1d13dc..22fd348fa 100644
--- a/Completion/Unix/Command/_man
+++ b/Completion/Unix/Command/_man
@@ -16,7 +16,7 @@
 _man() {
   local dirs expl mrd awk variant noinsert
   local -a context line state state_descr args modes
-  local -aU sects
+  local -aU sects _manpath
   local -A opt_args val_args sect_descs
 
   if [[ $service == man ]]; then
@@ -168,29 +168,36 @@ _man() {
   _arguments -s -S : $args '*::: :->man' && return 0
   [[ -n $state ]] || return 1
 
-  if (( ! $#_manpath )); then
-    local mp
-    mp=( ${(s.:.)$(manpath 2>/dev/null)} )
-    [[ "$mp" == *:* ]] && mp=( ${(s.:.)mp} )
-    if (( $#mp )); then
-      _manpath=( $mp )
-    elif (( $#manpath )); then
-      _manpath=( $manpath )
-    fi
-  fi
-
-  (( $#_manpath )) ||
-      _manpath=( /usr/man(-/) /(opt|usr)/(pkg|dt|share|X11R6|local)/(cat|)man(-/) )
-
   # Override man path
   [[ -n ${opt_args[-M]} ]] &&
   _manpath=( ${(s<:>)opt_args[-M]} )
 
+  # Restore cached man path to avoid $(manpath) if we can
+  if (( ! $#_manpath )); then
+    if (( ! $+_manpath_cache )); then
+      typeset -gHA _manpath_cache
+    fi
+    if [[ -z $_manpath_cache[$MANPATH] ]]; then
+      local mp
+      mp=( ${(s.:.)$(manpath 2>/dev/null)} )
+      [[ "$mp" == *:* ]] && mp=( ${(s.:.)mp} )
+      if (( $#mp )); then
+        _manpath_cache[$MANPATH]=${(j.:.)mp}
+      elif (( $#manpath )); then
+        _manpath_cache[$MANPATH]=$MANPATH
+      fi
+    fi
+    _manpath=( ${(s.:.)_manpath_cache[$MANPATH]} )
+  fi
+
   # Augment man path
   [[ $variant == (netbsd|openbsd)* ]] &&
   [[ -n ${opt_args[-m]} ]] &&
   _manpath+=( ${(s<:>)opt_args[-m]} )
 
+  (( $#_manpath )) ||
+      _manpath=( /usr/man(-/) /(opt|usr)/(pkg|dt|share|X11R6|local)/(cat|)man(-/) )
+
   # `sman' is the SGML manual directory for Solaris 7.
   # 1M is system administrator commands on SVR4