[PATCH 0/3] A set of _man fixes — explanation for 3(a)/3 and 3(b)/3

[PATCH 0/3] A set of _man fixes — explanation for 3(a)/3 and 3(b)/3

[Daniel Shahaf]

The third patch deserves an explanation.

The root problem is that «man getwpnam<_correct_word>» finds no matches.
(I pointed that in 38519.)

Patches 3(a) and 3(b) — the ones entitled "Support _correct_word" — are two
alternative fixes to that problem.  They are mutually exclusive.  Both of them
work (getpwnam and getspnam are offered as completions), but I'm not sure which
of them is preferable.

So, in short, I plan to push 1/3, 2/3, and either 3a or 3b, depending on feedback.

Cheers,

Daniel

[PATCH 1/3] _man: Drop (b): it's incorrect when $sect contains '|'.

[Daniel Shahaf]

---
 Completion/Unix/Command/_man | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/Completion/Unix/Command/_man b/Completion/Unix/Command/_man
index e892bb2..ef17ad8 100644
--- a/Completion/Unix/Command/_man
+++ b/Completion/Unix/Command/_man
@@ -37,7 +37,10 @@ _man() {
 
   mrd=(${^_manpath/\%L/${LANG:-En_US.ASCII}}/mandb(N))
 
-  # $sect is from the command line, the "3p" in "man 3p memcpy"
+  # $sect is from the command line, the "3p" in "man 3p memcpy".
+  #   It may also be a |-joined (and later in the code "()"-enclosed) list of
+  #   section names.
+  #   TODO: disentangle this to always be an array.
   # $sect_dirname is from the filesystem, the "3" in "/usr/share/man/man3"
   # These are used by _man_pages
   local sect sect_dirname
@@ -117,7 +120,7 @@ _man_pages() {
   fi
 
   pages=( ${(M)dirs:#*$sect_dirname/} )
-  compfiles -p pages '' '' "$matcher" '' dummy "*${(b)sect}*"
+  compfiles -p pages '' '' "$matcher" '' dummy "*${sect}*"
   pages=( ${^~pages}(N:t) )
 
   (($#mrd)) && pages[$#pages+1]=($(awk $awk $mrd))

[PATCH 2/3] _man: Fix two bugs when completing manpage filenames in separate-sections mode.

[Daniel Shahaf]

- No longer glob all files (the (-g)-less _path_files was virtually always called,
  by at least one of the multiple calls to _man_pages).

- Actually separate sections (by propagating $expl).
---
 Completion/Unix/Command/_man | 17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/Completion/Unix/Command/_man b/Completion/Unix/Command/_man
index ef17ad8..ffe53be 100644
--- a/Completion/Unix/Command/_man
+++ b/Completion/Unix/Command/_man
@@ -87,6 +87,13 @@ _man() {
       done
       (( ret )) || return 0
     done
+    ## To fall back to other sections' manpages when completing filenames, like
+    ## the 'else' codepath does:
+    #
+    # if (( ret )) && [[ $PREFIX$SUFFIX == */* ]]; then
+    #   sect_dirname=
+    #   _wanted manuals expl 'manual page' _man_pages && return
+    # fi
 
     return 1
   else
@@ -105,9 +112,13 @@ _man_pages() {
     # Easy way to test for versions of man that allow file names.
     # This can't be a normal man page reference.
     # Try to complete by glob first.
-    _path_files -g "*$suf" && return
-    _path_files
-    return
+    if [[ -n $sect_dirname ]]; then
+      _path_files -g "*.*$sect_dirname*(|.gz|.bz2|.Z|.lzma)" "$expl[@]"
+    else
+      _path_files -g "*$suf" "$expl[@]" && return
+      _path_files "$expl[@]"
+    fi
+    return $?
   fi
 
   zparseopts -E M+:=matcher

[PATCH 3(a)/3] _man: Support _correct_word.

[Daniel Shahaf]

The 'compfiles' call in «man 8 foo<TAB>» resulted in glob patterns of the form
«.../foo*», so the would-be targets of spelling correction would not have been
provided to the completion code by the globbing code.

Fix this by mimicking the only other callsite of compfiles, _path_files, which
passes a flag (${_comp_correct:+"-"}) that is only set by _approximate and
causes cfp_opt_pats() to be skipped.  This enables spelling correction to work,
however, it is unknown what side effects it may have.
---
 Completion/Unix/Command/_man |  2 +-
 Src/Zle/computil.c           | 15 +++++++++++++++
 2 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/Completion/Unix/Command/_man b/Completion/Unix/Command/_man
index ffe53be..3900cba 100644
--- a/Completion/Unix/Command/_man
+++ b/Completion/Unix/Command/_man
@@ -131,7 +131,7 @@ _man_pages() {
   fi
 
   pages=( ${(M)dirs:#*$sect_dirname/} )
-  compfiles -p pages '' '' "$matcher" '' dummy "*${sect}*"
+  compfiles -p${_comp_correct:+"-"} pages '' '' "$matcher" '' dummy "*${sect}*"
   pages=( ${^~pages}(N:t) )
 
   (($#mrd)) && pages[$#pages+1]=($(awk $awk $mrd))
diff --git a/Src/Zle/computil.c b/Src/Zle/computil.c
index e8f0a6f..0028ac1 100644
--- a/Src/Zle/computil.c
+++ b/Src/Zle/computil.c
@@ -4480,6 +4480,10 @@ cfp_matcher_pats(char *matcher, char *add)
     return add;
 }
 
+/*
+ * ### This function call is skipped by _approximate, so "opt" probably means "optimize".
+ */
+
 static void
 cfp_opt_pats(char **pats, char *matcher)
 {
@@ -4811,6 +4815,17 @@ cf_remove_other(char **names, char *pre, int *amb)
     return NULL;
 }
 
+/*
+ * SYNOPSIS:
+ *     1. compfiles -p  parnam1 parnam2 skipped matcher sdirs parnam3 varargs [..varargs]
+ *     2. compfiles -p- parnam1 parnam2 skipped matcher sdirs parnam3 varargs [..varargs]
+ *     3. compfiles -P  parnam1 parnam2 skipped matcher sdirs parnam3 
+ *
+ *     1. Set parnam1 to an array of patterns....
+ *     2. Like #1 but without calling cfp_opt_pats().  (This is only used by _approximate.)
+ *     3. Like #1 but varargs is implicitly set to  char *varargs[2] = { "*(-/)", NULL };.
+ */
+
 static int
 bin_compfiles(char *nam, char **args, UNUSED(Options ops), UNUSED(int func))
 {

[PATCH 3(b)/3] _man: Support _correct_word.

[Daniel Shahaf]

Since compfiles is undocumented, avoid its use altogether, replacing it by
a construct that blackbox analysis suggests to be equivalent.

The compfiles call being removed effected the following change (when
completing «man -S 8:1 getc<TAB>»):

    BEFORE THE CALL:
        typeset -a pages=( /home/daniel/prefix/zsh/share/man/man1/
                           /usr/share/man/man8/
                           /usr/share/man/man1/ )
    AFTER THE CALL:
        typeset -a pages=( '/home/daniel/prefix/zsh/share/man/man1/getc*(8|1)*'
                           '/usr/share/man/man8/getc*(8|1)*'
                           '/usr/share/man/man1/getc*(8|1)*' )

This patch effects the same transformation (modulo doubling the final slash).

Any -M parameter will be passed to compadd.
---
 Completion/Unix/Command/_man | 13 ++-----------
 1 file changed, 2 insertions(+), 11 deletions(-)

diff --git a/Completion/Unix/Command/_man b/Completion/Unix/Command/_man
index ffe53be..ae6ac38 100644
--- a/Completion/Unix/Command/_man
+++ b/Completion/Unix/Command/_man
@@ -103,7 +103,7 @@ _man() {
 }
 
 _man_pages() {
-  local matcher pages dummy sopt
+  local pages sopt
 
   # What files corresponding to manual pages can end in.
   local suf='.((?|<->*)(|.gz|.bz2|.Z|.lzma))'
@@ -121,17 +121,8 @@ _man_pages() {
     return $?
   fi
 
-  zparseopts -E M+:=matcher
-
-  if (( $#matcher )); then
-    matcher=( ${matcher:#-M} )
-    matcher="$matcher"
-  else
-    matcher=
-  fi
-
   pages=( ${(M)dirs:#*$sect_dirname/} )
-  compfiles -p pages '' '' "$matcher" '' dummy "*${sect}*"
+  pages=( ${^pages}/"*$sect${sect:+"*"}" );
   pages=( ${^~pages}(N:t) )
 
   (($#mrd)) && pages[$#pages+1]=($(awk $awk $mrd))

Re: [PATCH 3(b)/3] _man: Support _correct_word.

[Peter Stephenson]

On Thu, 4 Aug 2016 15:54:26 +0000
Daniel Shahaf <d.s@daniel.shahaf.name> wrote:
> Since compfiles is undocumented, avoid its use altogether, replacing it by
> a construct that blackbox analysis suggests to be equivalent.

This is likely to be saner, if it works.  compfiles was written to
replace an uncocumented chunk of _path_files with an equivalently
undocumented chunk of optimised C.

If you actually need more features of file completion, which you
probably don't as the user doesn't ultimately see the result as a file
match, going through _path_files would probably be less insane than
compfiles.

pws

Re: [PATCH 3(b)/3] _man: Support _correct_word.

[Bart Schaefer]

On Thu, Aug 4, 2016 at 9:08 AM, Peter Stephenson
<p.stephenson@samsung.com> wrote:
> On Thu, 4 Aug 2016 15:54:26 +0000
> Daniel Shahaf <d.s@daniel.shahaf.name> wrote:
>> Since compfiles is undocumented, avoid its use altogether, replacing it by
>> a construct that blackbox analysis suggests to be equivalent.
>
> This is likely to be saner, if it works.

I'll just point out that this proposed change means that _man_pages no
longer accepts the -M argument to add a matcher spec.  This might
cascade upward somehow, I haven't tried to figure that out.

Re: [PATCH 3(b)/3] _man: Support _correct_word.

[Bart Schaefer]

On Aug 4,  9:28am, Bart Schaefer wrote:
}
} I'll just point out that this proposed change means that _man_pages no
} longer accepts the -M argument

Duh.  I read only the response and not the original in detail.  -M is passed
on to compadd rather than being interpreted here.  Sorry for the noise.

Re: [PATCH 3(b)/3] _man: Support _correct_word.

[Daniel Shahaf]

Bart Schaefer wrote on Thu, Aug 04, 2016 at 09:28:44 -0700:
> On Thu, Aug 4, 2016 at 9:08 AM, Peter Stephenson
> <p.stephenson@samsung.com> wrote:
> > On Thu, 4 Aug 2016 15:54:26 +0000
> > Daniel Shahaf <d.s@daniel.shahaf.name> wrote:
> >> Since compfiles is undocumented, avoid its use altogether, replacing it by
> >> a construct that blackbox analysis suggests to be equivalent.
> >
> > This is likely to be saner, if it works.
> 
> I'll just point out that this proposed change means that _man_pages no
> longer accepts the -M argument to add a matcher spec.

Might you have misread the diff?

Before this patch, -M was handled twice: both by the zparseopts call and
by compadd.  With this patch, -M is only handled by compadd.

Empirically, matchspecs are honoured:

    % zstyle :completion:\* matcher-list ''  "r:|::=* r:|=*"
    % man X::L::Pat<TAB>
    <becomes>
    % man XML::Lib::Pattern

I tried to clarify this in the log message.

Cheers,

Daniel