Re: [PATCH] Completion: Improve _man (3)

[dana <dana@xxxxxxx>]

On 15 Jun 2018, at 09:39, Mikael Magnusson <mikachu@xxxxxxxxx> wrote:
>Please don't use () for 1 {} a b c style loops in actual scripts, it
>is not guaranteed to work.

Someone on IRC told me it was safe :(

Fixed though


On 15 Jun 2018, at 09:47, Daniel Shahaf <d.s@xxxxxxxxxxxxxxxxxx> wrote:
>This lists utime(2), utime(3), utf8(3perl), utf8(7), and utime.h(7posix), among
>others.  Should there be separate groups for each section, i.e., one group for
>each of (2 3 3perl 7 7posix)?  (This is orthogonal to dana's patch.)

Is your system set up so that each of those sub-sections has its own directory,
or are they pooled together into (in this case) 3 and 7? From my own testing,
they should be grouped separately in the former case, but not in the latter due
to the fact that (sub-)sections to complete are derived from the names of the
directories rather than the files.

That could be changed (in fact, assuming it introduces no inaccuracies, i think
it'd be preferable), but it would require reworking the _man_pages stuff, since
we don't know the page names until we get to the bottom of that function. I'll
think about that.


On 15 Jun 2018, at 09:55, Daniel Shahaf <d.s@xxxxxxxxxxxxxxxxxx> wrote:
>Now that you mention it, I think completion functions may not use shortloops
>syntax: compinit doesn't reset that option.

I'd considered that, but that syntax doesn't seem to be affected by short_loops.
That's why i thought it'd be OK.


On 15 Jun 2018, at 09:55, Daniel Shahaf <d.s@xxxxxxxxxxxxxxxxxx> wrote:
>That _still_ leaves us with a «for 1» loop, which I'm not actually sure is a
>documented syntax, but it's useful enough that I'd welcome a regression test
>for it.

Do you mean using 1 as the parameter name, or using for loops without the
`in ...` bit? The latter is specified by POSIX.


Unrelated, but i noticed an issue as i was testing. Maybe this is another case
of me fixing something that actually needs to be a certain way, but, with the
current function, if i create a page test.1dana and then do `man 1d <TAB>` i get
a bunch of results like this:

  perl5101delta perl5121delta perl5141delta ...

That doesn't seem desirable, does it? Is there any reason the glob pattern
shouldn't be restricted to *.$sect* (*.1d*) rather than *$sect* (*1d*)? The only
issue i can think of is that a few systems (like IRIX i believe) don't put the
(sub-)section in the file name at all, but those should already be broken, so...

Anticipating that the answer is no, i've added the dot in the attached patch —
let me know if it seems like a bad choice, though.

dana


diff --git a/Completion/Unix/Command/_man b/Completion/Unix/Command/_man
index 11c2fab7f..7fd92bac5 100644
--- a/Completion/Unix/Command/_man
+++ b/Completion/Unix/Command/_man
@@ -4,6 +4,11 @@
 # - Solaris is seemingly the only OS that doesn't allow the `man n page` syntax;
 #   you must use `man -s n page`
 # - We assume that Linux distributions are using either man-db or mandoc
+# - @todo Would be nice to support completing the initial operand as a section
+#   name (on non-Solaris systems)
+# - @todo We don't support the man-db syntax <name>.<section> (e.g., `ls.1`)
+# - @todo We don't support the man-db feature of 'sub-pages' — that is, treating
+#   pairs of operands like `git diff` as `git-diff`
 # - @todo Option exclusivity isn't super accurate
 # - @todo Solaris man accepts a single hyphen as the first option to disable
 #   paging (like AIX's -c); we don't support that
@@ -220,7 +225,15 @@ _man() {
   elif [[ $variant == (dragonfly|freebsd)* ]] && (( $+opt_args[-S] )); then
     noinsert=1
     sect=${opt_args[-S]//:/|}
-  elif (( CURRENT > 1 )) && [[ $variant != solaris* ]]; then
+  # It's only a small help, but, per man-db, we can avoid treating an initial
+  # operand like `8139too` as a section name by ensuring that only the first
+  # character is a digit. This doesn't do much for stuff like `2to3`, but we can
+  # at least special-case a few common patterns for now
+  elif
+    (( CURRENT > 1 )) &&
+    [[ $variant != solaris* ]] &&
+    [[ ${${(Q)words[1]}##(2to3|7z)*} == ([0-9](|[^0-9[:punct:]]*)|[lnopx]) ]]
+  then
     noinsert=1
     sect=$words[1]
   elif [[ -n ${sect:=$MANSECT} ]]; then
@@ -232,7 +245,25 @@ _man() {
 
   if [[ $sect = (<->*|[lnopx]) || $sect = *\|* ]]; then
     sects=( ${(s.|.)sect} )
-    dirs=( $^_manpath/(sman|man|cat)${^sects}(|.*)/ )
+
+    # Most man implementations support partial matching of a page's
+    # (sub-)section name — e.g., `3per` for `3perl`. The (sub-)section name may
+    # or may not correspond to the directory name (most systems combine
+    # sub-sections), but we'll assume that if it starts with a number and we're
+    # not on Solaris (which doesn't support this feature at all) that we can do
+    # a match against the leading number. This is irritating if you DO want the
+    # exact sub-section specified, but unfortunately there's no way to determine
+    # this programmatically — i guess we could add a style to control it
+    () {
+      for 1; do
+        if [[ $OSTYPE == solaris* || $1 != <->* ]]; then
+          dirs+=( $^_manpath/(sman|man|cat)$1(|.*)/ )
+        else
+          dirs+=( $^_manpath/(sman|man|cat)${1%%[^0-9]#}*/ )
+        fi
+      done
+    } $sects
+
     sect=${(j<|>)sects}
     [[ $sect == *'|'* ]] && sect="($sect)"
     awk="\$2 == \"$sect\" {print \$1}"
@@ -281,7 +312,7 @@ _man() {
       8        'maintenance commands and procedures'
       9        'kernel features'
       9lua     'Lua kernel bindings' # NetBSD
-      l        'local documentation' # AIX, etc.
+      l        'local documentation' # AIX, etc. — TCL on some systems?
       n        'new documentation' # AIX, etc.
       o        'old documentation' # AIX, etc.
       p        'public documentation' # AIX, etc.
@@ -380,14 +411,14 @@ _man_pages() {
   local pages sopt
 
   # What files corresponding to manual pages can end in.
-  local suf='.((?|<->*|ntcl)(|.gz|.bz2|.Z|.lzma))'
+  local suf='.((?|<->*|ntcl)(|.gz|.bz2|.z|.Z|.lzma))'
 
   if [[ $PREFIX$SUFFIX = */* ]]; then
     # 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.
     if [[ -n $sect_dirname ]]; then
-      _path_files -g "*.*$sect_dirname*(|.gz|.bz2|.Z|.lzma)" "$expl[@]"
+      _path_files -g "*.*$sect_dirname*(|.gz|.bz2|.z|.Z|.lzma)" "$expl[@]"
     else
       _path_files -g "*$suf" "$expl[@]" && return
       _path_files "$expl[@]"
@@ -396,7 +427,7 @@ _man_pages() {
   fi
 
   pages=( ${(M)dirs:#*$sect_dirname/} )
-  pages=( ${^pages}/"*$sect${sect:+"*"}" );
+  pages=( ${^pages}/"*${sect:+.$sect"*"}" )
   pages=( ${^~pages}(N:t) )
 
   (($#mrd)) && pages[$#pages+1]=($(awk $awk $mrd))