zsh-workers
 help / color / mirror / code / Atom feed
* [PATCH] Completion: Improve _man
@ 2018-06-10  6:06 dana
  2018-06-10 13:07 ` Oliver Kiddle
  0 siblings, 1 reply; 15+ messages in thread
From: dana @ 2018-06-10  6:06 UTC (permalink / raw)
  To: Zsh workers

Here's a *much* more complicated change. It enhances _man as follows:

* Complete all options to most major man variants
* More accurately match sections
* Better handle certain edge cases (see below)
* Show descriptions for sections (suggested by Mikael i think)

Notes:

* The function would previously use the provided section name as the leading
  part of a directory pattern, so that on Solaris for example if you gave it 3p
  it would also complete pages from 3pool, 3proc, &c. I couldn't think of a
  reason this functionality would be desirable (man won't show you a page from a
  section it's not in anyway), so i made it match exactly

* The function previously behaved kind of strangely when given multiple operands
  (where the first one is potentially an unknown section name). For example, if
  you do `man zz <TAB>` on macOS it just keeps completing the word 'fuzzy'
  because it wants to use zz as part of a glob to match page names. I changed
  that so that if the section name (given either as the first operand or as an
  argument to -s/-S) doesn't seem valid, it's treated as a page name, and
  subsequent completion possibilities are thus returned from all sections

  This can still be a problem — `man 2to3 <TAB>` doesn't work well, for example,
  because the function thinks 2to3 looks like a (Solaris-style) section name. I
  know this can be fixed, but see my last two notes :/

* It looks like someone put some special effort into making the function at
  least partially usable on AIX, so i tried to account for that platform as
  well... but i don't actually have access to it myself, so the extent of my
  testing there was just OSTYPE=aix

* I chose sections to describe pretty arbitrarily. Solaris variants have 9034234
  different sections and i didn't want to enumerate every single one, but some
  of them seem like they might come up a lot in regular use. It may be nice in
  the future to give separate tags to the described and undescribed ones, so
  that navigation is a little nicer for people with `group-name ''`

* There were a bunch of options whose purpose wasn't very clear to me, so i
  couldn't complete them as well as i wanted to. I put @todos where this was the
  case. If anyone knows how they actually work, feel free to change them
  accordingly

* There are some bits that were (or are now) redundant or otherwise not great —
  see the todo note someone left about the sects array for example — but i just
  left them alone because i was afraid of breaking something

* Could someone please review the way i return from _arguments? I don't think
  it's correct (because it doesn't account for prefix-needed?), but i've been
  looking at this for too long

I've tested this on several different platforms, but it's still a pretty major
alteration to a function that i imagine sees a lot of traffic, so i'm a little
nervous about it. If someone else could try it out that might be a good idea

dana


diff --git a/Completion/Unix/Command/_man b/Completion/Unix/Command/_man
index 67810e1dc..14e9e75b5 100644
--- a/Completion/Unix/Command/_man
+++ b/Completion/Unix/Command/_man
@@ -1,16 +1,169 @@
 #compdef man apropos whatis
 
+# Notes:
+# - 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 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
+# - @todo Linux apropos/whatis take options; we don't complete them yet
+
 _man() {
-  local dirs expl mrd awk
+  local dirs expl mrd awk variant noinsert
+  local -a context line state state_descr args modes
+  local -aU sects
+  local -A opt_args val_args sect_descs
 
-  if (( $words[(I)-M] == (( $CURRENT - 1 )) )); then
-    _directories && return 0
-  fi
+  if [[ $service == man ]]; then
+    # We'll treat all mandoc-based systems (Alpine, various Illumos distros,
+    # etc.) as OpenBSD
+    _pick_variant -r variant openbsd='-S subsection' $OSTYPE ---
+
+    modes=(
+      -f -K -k -l -R -w -W
+      --apropos
+      --global-apropos
+      --local-file
+      --location
+      --location-cat
+      --recode
+      --whatis
+      --where
+      --where-cat
+    )
+    [[ $variant == darwin* ]] && modes+=( -t )
+
+    args=(
+      "(${(j< >)modes})"{-f,--whatis}'[display short description (like whatis)]'
+      "(${(j< >)modes})"{-k,--apropos}'[search for keyword (like apropos)]'
+      '(-M --manpath)'{-M+,--manpath=}'[specify manual search path]:manual search path:_sequence -s\: _directories'
+    )
+    if [[ $variant == (darwin|dragonfly|freebsd|linux)* ]]; then
+      args+=(
+        '(-a -S -s --all --sections)'{-a,--all}'[display all matching pages]'
+        '(-P --pager)'{-P+,--pager=}'[specify output pager]:pager:_path_commands'
+        # @todo Could enumerate these
+        '(-p --preprocessor)'{-p+,--preprocessor=}'[specify roff preprocessor sequence]:preprocessor sequence'
+      )
+    else
+      args+=( '(-s)-a[display all matching pages]' )
+    fi
+    [[ $variant == (aix|solaris)* ]] || args+=(
+      '(-C --config-file)'{-C+,--config-file=}'[specify configuration file]:configuration file:_files'
+      "(${(j< >)modes})"{-w,--path,--where}'[display file locations]'
+    )
+    [[ $variant == (aix|netbsd|openbsd)* ]] || args+=(
+      # @todo FreeBSD allows this to be given multiple times
+      '(-d --debug)'{-d,--debug}'[display debugging information]'
+    )
+    [[ $variant == (darwin|dragonfly|freebsd|linux|solaris|aix)* ]] && args+=(
+      '(-7 -H -t --ascii --html --troff)'{-t,--troff}'[format man page using troff]'
+    )
+    [[ $variant == (darwin|linux)* ]] && args+=(
+      "(${(j< >)modes})"{-K,--global-apropos}'[search for keyword in all pages]'
+      '(-m --systems)'{-m+,--systems=}'[search manual of specified system]:operating system'
+    )
+    [[ $variant == (darwin|dragonfly|freebsd)* ]] && args+=(
+      '(: -)-h[display help information]'
+      '(-a)-S+[specify manual sections to search]: :->sects'
+    )
+    [[ $variant == (dragonfly|freebsd)* ]] && args+=(
+      # @todo Could enumerate these
+      '-m[search manual of specified architecture]:architecture'
+      '-o[use non-localized man pages]'
+    )
+    [[ $variant == (netbsd|openbsd)* ]] && args+=(
+      '-c[disable paging]'
+      '-m[augment manual search path]:manual search path:_sequence -s\: _directories'
+      '(-a)-s+[specify manual section to search]: :->sects'
+    )
+    [[ $variant == linux* ]] && args+=(
+      '(: -)'{-\?,--help}'[display help information]'
+      '(-7 -t -H -T -Z --ascii --html --troff --troff-device --ditroff)'{-7,--asci}'[translate man pages for 7-bit terminal]'
+      '(-D --default)'{-D,--default}'[reset man to default options]'
+      # @todo Could enumerate these
+      '(-E --encoding)'{-E+,--encoding=}'[specify output encoding]:encoding'
+      '(-e --extension)'{-e+,--extension=}'[specify sub-extension]:sub-extension'
+      '(-H --html)'{-H-,--html=-}'[produce HTML output for specified browser]::Web browser:_path_commands'
+      '(-i -I --ignore-case --match-case)'{-i,--ignore-case}'[search case-insensitively]'
+      '(-i -I --ignore-case --match-case)'{-I,--match-case}'[search case-sensitively]'
+      '(-L --locale)'{-L+,--locale=}'[specify locale]:locale:_locales'
+      "(${(j< >)modes})"{-l+,--local-file=}'[format and display specified file]:*:::manual file:_files'
+      "!(${(j< >)modes})"{--location,--location-cat}
+      '--names-only[match only page names (with --regex or --wildcard)]'
+      '(--nh --no-hyphenation)'{--nh,--no-hyphenation}'[disable hyphenation]'
+      '(--nj --no-justification)'{--nj,--no-justification}'[disable justification]'
+      '--no-subpages[do not combine pairs of page names into single page name]'
+      # @todo Could enumerate these
+      "(${(j< >)modes})"{-R+,--recode=}'[output man page in specified encoding]:encoding'
+      '(-r --prompt)'{-r+,--prompt=}'[specify prompt for less]:less prompt'
+      '(-a --all --wildcard)--regex[treat page name as regular expression]'
+      '(-a -S -s --all --sections)'{-S+,-s+,--sections=}'[specify manual sections to search]: :->sects'
+      # @todo Could enumerate these
+      '(-T -t --troff --troff-device)'{-T-,--troff-device=-}'[specify roff output device]::roff output device'
+      '(-u --update)'{-u,--update}'[update database caches]'
+      '(: -)--usage[display brief usage information]'
+      '(: -)'{-V,--version}'[display version information]'
+      "(${(j< >)modes})"{-W,--where-cat}'[display cat file locations]'
+      '--warnings=[enable specified groff warnings]:groff warnings'
+      '(-a --all --regex)--wildcard[treat page name as shell glob]'
+      # @todo Could enumerate these
+      '(-X --gxditview)'{-X-,--gxditview=-}'[display output in gxditview using specified DPI (default: 75)]::DPI'
+      # @todo Post-process how?
+      '(-t --troff -Z --ditroff)'{-Z,--ditroff}'[post-process output for chosen device]'
+    )
+    [[ $variant == darwin* ]] && args+=(
+      # We use _files here because browsers are usually in /Applications, which
+      # typically isn't in PATH
+      '-B+[specify browser to use for HTML files]:Web browser:_files'
+      '-c[reformat source man page]'
+      # @todo -d should be exclusive with this above
+      '(-d)-D[display man page along with debugging information]'
+      '(-D -F --preformat)'{-F,--preformat}'[format man page only (do not display)]'
+      '-H+[specify command to render HTML as text]:HTML pager:_path_commands'
+      # --help and --version are undocumented but functional
+      '(: -)--help[display help information]'
+      # -s is also undocumented; it's provided for compatibility with Solaris
+      '!(-S)-s+: :->sects'
+      '(: -)'{-v,--version}'[display version information]'
+      "(${(j< >)modes})-W[display file locations, one per line, with no other information]"
+    )
+    [[ $variant == netbsd* ]] && args+=(
+      '-h[display only synopsis lines]'
+      '(: -)-p[display manual search path]'
+      '-S+[display only man pages with file names matching specified string]:search string'
+    )
+    [[ $variant == openbsd* ]] && args+=(
+      "(${(j< >)modes})-l+[format and display specified file]:*:::manual file:_files"
+      # @todo Could enumerate these
+      '-S[search manual of specified architecture]:architecture'
+    )
+    [[ $variant == solaris* ]] && args+=(
+      "(${(j< >)modes})-l[display file locations]"
+      '-r[format man page only (do not display)]'
+      '(-a)-s+[specify manual sections to search]: :->sects'
+      # @todo Does this in fact want a file path?
+      '-T+[format man page using specified macro package]:macro package:_files'
+    )
+    [[ $variant == aix* ]] && args+=(
+      '-c[display man page using cat]'
+      '-F[display only first matching entry]'
+      '-m[only search paths specified by -M/MANPATH]'
+      '-r[search remotely]'
+    )
 
-  if [[ $service == man ]] && (( $words[(I)-l] + $words[(I)--local-file] )); then
-    _files || return 0
+    # Strip (most) long options from non-Linux platforms
+    if [[ $variant == darwin* ]]; then
+      args=( ${(M)args:#((#s)|*\))(\*|)(-[^-]|--(help|path|pref|vers))*} )
+    elif [[ $variant != linux* ]]; then
+      args=( ${(M)args:#((#s)|*\))(\*|)-[^-]*} )
+    fi
   fi
 
+  _arguments -s -S : $args '*::: :->man' && return 0
+  [[ -n $state ]] || return 1
+
   if (( ! $#_manpath )); then
     local mp
     mp=( ${(s.:.)$(manpath 2>/dev/null)} )
@@ -23,14 +176,16 @@ _man() {
   fi
 
   (( $#_manpath )) ||
-      _manpath=( /usr/man(-/) /(opt|usr)/(pkg|dt|share|X11R6|local)/(cat|)man(-/) )
+  _manpath=( /usr/man(-/) /(opt|usr)/(pkg|dt|share|X11R6|local)/(cat|)man(-/) )
 
-  integer index=$words[(I)-M]
-  if (( index )); then
-    local opt
-    opt=$words[index+1]
-    _manpath=($opt)
-  fi
+  # Override man path
+  [[ -n ${opt_args[-M]} ]] &&
+  _manpath=( ${(s<:>)opt_args[-M]} )
+
+  # Augment man path
+  [[ $variant == (netbsd|openbsd)* ]] &&
+  [[ -n ${opt_args[-m]} ]] &&
+  _manpath+=( ${(s<:>)opt_args[-m]} )
 
   # `sman' is the SGML manual directory for Solaris 7.
   # 1M is system administrator commands on SVR4
@@ -44,52 +199,158 @@ _man() {
   # $sect_dirname is from the filesystem, the "3" in "/usr/share/man/man3"
   # These are used by _man_pages
   local sect sect_dirname
-  if [[ $OSTYPE = solaris* ]]; then
-    sect=${${words[(R)-s*]#-s}:-$words[$words[(i)-s]+1]}
-    sect="${sect//,/|}"
-  elif [[ -n ${sect:=$words[$words[(i)-S]+1]} || -n ${sect:=$MANSECT} ]]; then
-    sect="${sect//:/|}"
-    sect="${sect//,/|}"
-  elif (( CURRENT > 2 )); then
-    case $words[2] in
-      (-a) sect='*';;
-      (-*) ;;
-      (*)  sect=$words[2];;
-    esac
+
+  # Take care: We can't use the sections from these options until we've finished
+  # completing them; otherwise (e.g.) -s1:<TAB> will give no results
+  if
+    [[ $service != man ]] || [[ $state == sects ]] || (( $+opt_args[-a] ))
+  then
+    sect='*'
+  elif
+    [[ $variant == (darwin|linux)* ]] &&
+    [[ -n ${opt_args[(i)-S|-s|--sections]} ]]
+  then
+    noinsert=1
+    sect=${opt_args[${opt_args[(i)-S|-s|--sections]}]//[:,]/|}
+  elif
+    [[ $variant == (netbsd|openbsd|solaris)* ]] && (( $+opt_args[-s] ))
+  then
+    noinsert=1
+    sect=${opt_args[-s]//,/|}
+  elif [[ $variant == (dragonfly|freebsd)* ]] && (( $+opt_args[-S] )); then
+    noinsert=1
+    sect=${opt_args[-S]//:/|}
+  elif (( CURRENT > 1 )) && [[ $variant != solaris* ]]; then
+    noinsert=1
+    sect=$words[1]
+  elif [[ -n ${sect:=$MANSECT} ]]; then
+    sect=${sect//:/|}
   fi
 
-  if [[ $sect = (<->*|1M|l|n) || $sect = *\|* ]]; then
-    () {
-      local -a sects=( ${(s.|.)sect} )
-      if [[ $sect != (l|n) ]]; then
-        sects=( ${sects%%[^0-9]#} )
-      fi
-      dirs=( $^_manpath/(sman|man|cat)${^sects}*/ )
-    }
-    if [[ $sect == *\|* ]]; then sect="($sect)"; fi
+  # Colons may have been escaped
+  sect=${(Q)sect}
+
+  if [[ $sect = (<->*|[lnopx]) || $sect = *\|* ]]; then
+    sects=( ${(s.|.)sect} )
+    dirs=( $^_manpath/(sman|man|cat)${^sects}/ )
+    sect=${(j<|>)sects}
+    [[ $sect == *'|'* ]] && sect="($sect)"
     awk="\$2 == \"$sect\" {print \$1}"
   else
+    sect=
     dirs=( $^_manpath/(sman|man|cat)*/ )
     awk='{print $1}'
   fi
+
+  # Ignore directories with no pages inside
+  dirs=( ${^dirs}(#qFN) )
+
   # Solaris 11 and on have a man-index directory that doesn't contain manpages
   dirs=( ${dirs:#*/man-index/} )
-  if [[ $OSTYPE = solaris* && ( $words[CURRENT] = -s* || $words[CURRENT-1] == -s ) ]]; then
-    [[ $words[CURRENT] = -s* ]] && compset -P '-s'
-    sects=( ${(o)${dirs##*(man|cat)}%/} )
-    _wanted sections expl 'section' compadd -a sects
-  elif zstyle -t ":completion:${curcontext}:manuals" separate-sections; then
-    typeset -U sects
-    local ret=1
+  sects=( ${(o)${dirs##*(man|cat)}%/} )
+
+  # If we've got this far, we can build our look-up table for descriptions of
+  # the more common sections. Unless otherwise labelled, the more specific ones
+  # come from Solaris or one of its variants
+  (( $#sects )) && () {
+    sect_descs=(
+      0        'library headers'
+      1        'general commands'
+      1cups    'CUPS commands'
+      1m       'maintenance commands'
+      1openssl 'OpenSSL commands'
+      2        'system calls'
+      3        'library functions'
+      3c       'C library functions'
+      3curses  'curses library functions'
+      3elf     'ELF library functions'
+      3f       'Fortran library functions'
+      3lua     'Lua features' # NetBSD
+      3mail    'mailbox library functions'
+      3openssl 'OpenSSL library functions'
+      3pam     'PAM library functions'
+      3pool    'pool configuration library functions'
+      3proc    'process control library functions'
+      3x11     'Xlib functions'
+      3xcurses 'curses library functions [X/Open]'
+      4        'devices and drivers'
+      5        'file formats and conventions'
+      3openssl 'OpenSSL configuration files'
+      6        'games'
+      7        'miscellanea'
+      8        'maintenance commands and procedures'
+      9        'kernel features'
+      9lua     'Lua kernel bindings' # NetBSD
+      l        'local documentation' # AIX, etc.
+      n        'new documentation' # AIX, etc.
+      o        'old documentation' # AIX, etc.
+      p        'public documentation' # AIX, etc.
+      x        'X11 features'
+    )
+
+    # Add POSIX variants
+    for 1 in ${(k)sect_descs}; do
+      [[ $1 == <-> ]] || continue
+      sect_descs+=( "${1}p" "${sect_descs[$1]} [POSIX]" )
+    done
+
+    # Add OS-specific stuff that's too risky for or overrides the general list
+    [[ $OSTYPE == darwin*  ]] && sect_descs+=( n 'Tcl/Tk features' )
+    [[ $OSTYPE == openbsd* ]] && sect_descs+=( 3p 'Perl features' )
+    [[ $OSTYPE == solaris* ]] && sect_descs+=(
+      1t  'Tcl/Tk features'
+      3m  'mathematical library functions'
+      4   'file formats and conventions'
+      5   'miscellanea'
+      7   'special files'
+      7d  'devices'
+      7fs 'file systems'
+      7i  'ioctl requests'
+      7m  'STREAMS modules'
+      7p  'protocols'
+      9e  'driver entry points'
+      9f  'driver functions'
+      9p  'driver properties'
+      9s  'driver data structures'
+    )
+  }
 
-    sects=( ${(o)${dirs##*(man|cat)}%/} )
+  [[ $state == sects ]] && {
+    local s
+    local -a specs
+
+    (( $#sects )) || {
+      _message 'manual section'
+      return 1
+    }
+
+    # Build specs from descriptions
+    for s in $sects; do
+      specs+=( "${s}[${(b)sect_descs[$s]}]" )
+    done
+
+    if [[ $variant == (darwin|dragonfly|freebsd|linux)* ]]; then
+      _values -s : 'manual section' $specs
+    elif [[ $variant == solaris* ]]; then
+      _values -s , 'manual section' $specs
+    else
+      _values 'manual section' $specs
+    fi
+    return
+  }
+
+  if zstyle -t ":completion:${curcontext}:manuals" separate-sections; then
+    local d ret=1
 
     (( $#sects )) || return 1
 
     _tags manuals.${^sects}
     while _tags; do
       for sect_dirname in $sects; do
-        _requested manuals.$sect_dirname expl "manual page, section $sect_dirname" _man_pages &&
+        d=$sect_dirname
+        (( $+sect_descs[$d] )) && d+=" (${sect_descs[$d]})"
+
+        _requested manuals.$sect_dirname expl "manual page, section $d" _man_pages &&
             ret=0
       done
       (( ret )) || return 0
@@ -113,7 +374,7 @@ _man_pages() {
   local pages sopt
 
   # What files corresponding to manual pages can end in.
-  local suf='.((?|<->*)(|.gz|.bz2|.Z|.lzma))'
+  local suf='.((?|<->*|ntcl)(|.gz|.bz2|.Z|.lzma))'
 
   if [[ $PREFIX$SUFFIX = */* ]]; then
     # Easy way to test for versions of man that allow file names.
@@ -138,8 +399,8 @@ _man_pages() {
   # beginning with .<->: that handles problem cases like files called
   # `POSIX.1.5'.
 
-  [[ $OSTYPE = solaris* ]] && sopt='-s '
-  if ((CURRENT > 2)) ||
+  [[ $variant = solaris* ]] && sopt='-s '
+  if ((CURRENT > 1 || noinsert)) ||
       ! zstyle -t ":completion:${curcontext}:manuals.$sect_dirname" insert-sections
   then
     compadd "$@" - ${pages%$~suf}


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] Completion: Improve _man
  2018-06-10  6:06 [PATCH] Completion: Improve _man dana
@ 2018-06-10 13:07 ` Oliver Kiddle
  2018-06-10 14:02   ` dana
  0 siblings, 1 reply; 15+ messages in thread
From: Oliver Kiddle @ 2018-06-10 13:07 UTC (permalink / raw)
  To: dana; +Cc: Zsh workers

dana wrote:
> Here's a *much* more complicated change. It enhances _man as follows:

This looks like a big improvement, thanks.

> * Complete all options to most major man variants
> * More accurately match sections
> * Better handle certain edge cases (see below)
> * Show descriptions for sections (suggested by Mikael i think)

Those look useful.

> Notes:
>
> * The function would previously use the provided section name as the leading
>   part of a directory pattern, so that on Solaris for example if you gave it 3p
>   it would also complete pages from 3pool, 3proc, &c. I couldn't think of a
>   reason this functionality would be desirable (man won't show you a page from a
>   section it's not in anyway), so i made it match exactly

Is this the line where you removed * from:
  dirs=( $^_manpath/(sman|man|cat)${^sects}*/ )

I think I've seen systems with directories named man1.Z/cat1.Z etc.
HP/UX perhaps. Perhaps also locales have occured such as man3f.fi_FI
I suspect the * was to handle something like that rather than for the
undesirable functionality you describe.

> * I chose sections to describe pretty arbitrarily. Solaris variants have 9034234
>   different sections and i didn't want to enumerate every single one, but some

I'm only seeing 270 sections, still too many to start enumerating them.

It looks like the sections will be changing with Solaris 11.4:

https://blogs.oracle.com/solaris/normalizing-man-page-section-numbers-in-solaris-114-v2

>   of them seem like they might come up a lot in regular use. It may be nice in
>   the future to give separate tags to the described and undescribed ones, so
>   that navigation is a little nicer for people with `group-name ''`

Am not so sure about that. Navigation seemed fine without. And _describe
doesn't currently support a mix of tags.

> * Could someone please review the way i return from _arguments? I don't think
>   it's correct (because it doesn't account for prefix-needed?), but i've been
>   looking at this for too long

Indeed, with the prefix-needed style set, _arguments adds the options
and then we should also add man pages in the state. (I use this with
_approximate to handle mistakes in the prefix itself). As it is, by
doing a return, the function ends before the state can be handled.
Usually, this is solved with a ret variable, but care would be needed
not to disturb the existing ret variable used later in the function. A
lazy alternative is to use $compstate[nmatches]

> I've tested this on several different platforms, but it's still a pretty major
> alteration to a function that i imagine sees a lot of traffic, so i'm a little
> nervous about it. If someone else could try it out that might be a good idea

For the most part, this looks great. I'll push it as-is for now. Any
further tweaks can then be done as incrementals diffs.

> +   '(-X --gxditview)'{-X-,--gxditview=-}'[display output in gxditview using specified DPI (default: 75)]::DPI' 

It's a minor thing but I find it more useful to put hints on the
arguments (units, defaults) etc in the heading for them.

  '(-X --gxditview)'{-X-,--gxditview=-}'[display output in gxditview optionally using specified resolution]::resolution (DPI) [75]'

> +  [[ $state == sects ]] && {
> +    local s
> +    local -a specs
> +
> +    (( $#sects )) || {
> +      _message 'manual section'

This should use _message -e and a tag so that it honours the user's
setting of the format style with the descriptions tag. And, to allow
the user to add some matches with the fake style.

> +      return 1
> +    }
> +
> +    # Build specs from descriptions
> +    for s in $sects; do
> +      specs+=( "${s}[${(b)sect_descs[$s]}]" )
> +    done

This results in all sections for which we don't have a description
getting an empty description. The descriptions that we have are then
pushed right over to the right like this:

3zonestat    5openssl  5oldap   4b                                          
7openssl     8oldap    7ipp            --                                     
1openssl                               -- OpenSSL commands                    
1t                                     -- Tcl/Tk features  

Try:
  _values x {a,b,c,d,e,f}'[]' 'g[description]'
vs.
  _values x {a,b,c,d,e,f} 'g[description]'

I'd suggest using _sequence wrapping _describe instead.
Initialise sect_descs in _describe format to begin with and then it can
be filtered like this:

  sect_descs=( 1:one 2:two )
  sects=( 1 2 3 4 )
  remove=( ${sect_descs%%:*} )
  sect_descs+=( ${sects:|remove} )

_describe also makes it easier to apply a more useful tag than just
values which can match the one from _message -e mentioned above.

Oliver


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] Completion: Improve _man
  2018-06-10 13:07 ` Oliver Kiddle
@ 2018-06-10 14:02   ` dana
  2018-06-11 10:48     ` [PATCH] Completion: Improve _man (2) dana
  0 siblings, 1 reply; 15+ messages in thread
From: dana @ 2018-06-10 14:02 UTC (permalink / raw)
  To: Zsh workers; +Cc: Oliver Kiddle

On 10 Jun 2018, at 08:07, Oliver Kiddle <okiddle@yahoo.co.uk> wrote:
>Is this the line where you removed * from:
> dirs=( $^_manpath/(sman|man|cat)${^sects}*/ )
>
>I think I've seen systems with directories named man1.Z/cat1.Z etc.
>HP/UX perhaps. Perhaps also locales have occured such as man3f.fi_FI
>I suspect the * was to handle something like that rather than for the
>undesirable functionality you describe.

It is, and that would make sense. If that is indeed the reason the pattern was
there, maybe it would be more accurate like this?

  dirs=( $^_manpath/(sman|man|cat)${^sects}(|.*)/ )

But then i think we'd need to strip the .* back out afterwards too.

On 10 Jun 2018, at 08:07, Oliver Kiddle <okiddle@yahoo.co.uk> wrote:
>It looks like the sections will be changing with Solaris 11.4:
>
>https://blogs.oracle.com/solaris/normalizing-man-page-section-numbers-in-solaris-114-v2

lol. Well... that's time well spent then.

Also i guess that change is going to introduce another issue like the current
_shutdown function has where relying on OSTYPE alone produces false positives
(because all of the Solaris variants report solaris2.11 or whatever, but some of
them are quite different from each other — now they'll have different man-page
structures).

>This results in all sections for which we don't have a description
>getting an empty description. The descriptions that we have are then
>pushed right over to the right like this:

Yeahhh. I identified that problem at some point, but then i forgot about the
actual implications. I like the _describe solution.

I'll try to fix any remaining issues with this later. Thanks!

dana


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] Completion: Improve _man (2)
  2018-06-10 14:02   ` dana
@ 2018-06-11 10:48     ` dana
  2018-06-14  9:50       ` Daniel Shahaf
  0 siblings, 1 reply; 15+ messages in thread
From: dana @ 2018-06-11 10:48 UTC (permalink / raw)
  To: Zsh workers; +Cc: Oliver Kiddle

Hi Oliver,

Here are some fixes per your earlier feedback.

I updated the section-directory glob to allow a compression/locale 'extension';
as i said, i've never actually seen this myself, but your theory seems
plausible, and it shouldn't hurt anything on other systems.

I switched to _describe for completing sections, though i didn't use the exact
suggestion you gave because sect_descs is used farther down in the function and
i would have had to change stuff there as well.

I did NOT change the _arguments return thing, because frankly i don't really
understand the expected results for that prefix-needed configuration, and i
didn't want to break anything down-function in my ignorance. :/

dana


diff --git a/Completion/Unix/Command/_man b/Completion/Unix/Command/_man
index 4cba9d625..11c2fab7f 100644
--- a/Completion/Unix/Command/_man
+++ b/Completion/Unix/Command/_man
@@ -109,7 +109,7 @@ _man() {
       '--warnings=[enable specified groff warnings]:groff warnings'
       '(-a --all --regex)--wildcard[treat page name as shell glob]'
       # @todo Could enumerate these
-      '(-X --gxditview)'{-X-,--gxditview=-}'[display output in gxditview using specified DPI (default: 75)]::DPI'
+      '(-X --gxditview)'{-X-,--gxditview=-}'[display output in gxditview using specified DPI]::resolution (DPI) [75]'
       # @todo Post-process how?
       '(-t --troff -Z --ditroff)'{-Z,--ditroff}'[post-process output for chosen device]'
     )
@@ -232,7 +232,7 @@ _man() {
 
   if [[ $sect = (<->*|[lnopx]) || $sect = *\|* ]]; then
     sects=( ${(s.|.)sect} )
-    dirs=( $^_manpath/(sman|man|cat)${^sects}/ )
+    dirs=( $^_manpath/(sman|man|cat)${^sects}(|.*)/ )
     sect=${(j<|>)sects}
     [[ $sect == *'|'* ]] && sect="($sect)"
     awk="\$2 == \"$sect\" {print \$1}"
@@ -247,7 +247,7 @@ _man() {
 
   # Solaris 11 and on have a man-index directory that doesn't contain manpages
   dirs=( ${dirs:#*/man-index/} )
-  sects=( ${(o)${dirs##*(man|cat)}%/} )
+  sects=( ${(o)${${dirs##*(man|cat)}%.*}%/} )
 
   # If we've got this far, we can build our look-up table for descriptions of
   # the more common sections. Unless otherwise labelled, the more specific ones
@@ -297,6 +297,11 @@ _man() {
     # Add OS-specific stuff that's too risky for or overrides the general list
     [[ $OSTYPE == darwin*  ]] && sect_descs+=( n 'Tcl/Tk features' )
     [[ $OSTYPE == openbsd* ]] && sect_descs+=( 3p 'Perl features' )
+    # @todo Oracle Solaris 11.4 adopts the BSD/Linux structure, making many of
+    # these inaccurate — this should be handled accordingly in the future. If
+    # OSTYPE isn't helpful (since other Solaris descendants may not follow
+    # suit), we could perhaps use the presence of SysV-style sections under
+    # _manpath as the determinant
     [[ $OSTYPE == solaris* ]] && sect_descs+=(
       1t  'Tcl/Tk features'
       3m  'mathematical library functions'
@@ -320,21 +325,22 @@ _man() {
     local -a specs
 
     (( $#sects )) || {
-      _message 'manual section'
+      _message -e sections 'manual section'
       return 1
     }
 
     # Build specs from descriptions
     for s in $sects; do
-      specs+=( "${s}[${(b)sect_descs[$s]}]" )
+      specs+=( "${s}:${(b)sect_descs[$s]}" )
     done
+    specs=( ${specs%:} )
 
     if [[ $variant == (darwin|dragonfly|freebsd|linux)* ]]; then
-      _values -s : 'manual section' $specs
+      _sequence -s : _describe -t sections 'manual section' specs
     elif [[ $variant == solaris* ]]; then
-      _values -s , 'manual section' $specs
+      _sequence -s , _describe -t sections 'manual section' specs
     else
-      _values 'manual section' $specs
+      _describe -t sections 'manual section' specs
     fi
     return
   }


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] Completion: Improve _man (2)
  2018-06-11 10:48     ` [PATCH] Completion: Improve _man (2) dana
@ 2018-06-14  9:50       ` Daniel Shahaf
  2018-06-14 10:20         ` dana
  0 siblings, 1 reply; 15+ messages in thread
From: Daniel Shahaf @ 2018-06-14  9:50 UTC (permalink / raw)
  To: dana; +Cc: Zsh workers

dana wrote on Mon, Jun 11, 2018 at 05:48:33 -0500:
> +++ b/Completion/Unix/Command/_man
> @@ -232,7 +232,7 @@ _man() {
> -    dirs=( $^_manpath/(sman|man|cat)${^sects}/ )
> +    dirs=( $^_manpath/(sman|man|cat)${^sects}(|.*)/ )

On Debian, «man 3per utf8» displays the man page utf8(3perl), which this change
breaks.

Sorry for the late response (didn't have time until now).


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] Completion: Improve _man (2)
  2018-06-14  9:50       ` Daniel Shahaf
@ 2018-06-14 10:20         ` dana
  2018-06-15 13:59           ` [PATCH] Completion: Improve _man (3) dana
  0 siblings, 1 reply; 15+ messages in thread
From: dana @ 2018-06-14 10:20 UTC (permalink / raw)
  To: Daniel Shahaf; +Cc: Zsh workers

On 14 Jun 2018, at 04:50, Daniel Shahaf <d.s@daniel.shahaf.name> wrote:
>On Debian, «man 3per utf8» displays the man page utf8(3perl), which this change
>breaks.

Oh, you're right. I was testing Solaris because it actually breaks them out into
separate directories — and it doesn't seem to work there, as i said — but the
Darwin and *BSD man tools have the same behaviour as man-db, when you can find
(or create) examples of it.

It was only really irritating on Solaris anyway, so maybe i'll just put it back
for everything else? Will experiment more and submit a patch later. Thanks!

dana


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] Completion: Improve _man (3)
  2018-06-14 10:20         ` dana
@ 2018-06-15 13:59           ` dana
  2018-06-15 14:05             ` Daniel Shahaf
  0 siblings, 1 reply; 15+ messages in thread
From: dana @ 2018-06-15 13:59 UTC (permalink / raw)
  To: Daniel Shahaf; +Cc: Zsh workers

Round 3, i suppose:

* Re-add the partial section-name matching functionality that Daniel mentioned
* More accurately determine whether an initial operand could be a section name
* Support .z compression extension (per man-db)
* Add some more documentation

Daniel (or anyone), if you have time, could you see if this works as you'd
expect?

dana


diff --git a/Completion/Unix/Command/_man b/Completion/Unix/Command/_man
index 11c2fab7f..10395de0e 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
@@ -231,8 +244,24 @@ _man() {
   sect=${(Q)sect}
 
   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 {
+      if [[ $OSTYPE == solaris* || $1 != <->* ]]; then
+        sects+=( $1 )
+        dirs+=( $^_manpath/(sman|man|cat)$1(|.*)/ )
+      else
+        sects+=( ${1%%[^0-9]#} )
+        dirs+=( $^_manpath/(sman|man|cat)${1%%[^0-9]#}*/ )
+      fi
+    } ${(s.|.)sect}
+
     sect=${(j<|>)sects}
     [[ $sect == *'|'* ]] && sect="($sect)"
     awk="\$2 == \"$sect\" {print \$1}"
@@ -281,7 +310,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 +409,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[@]"


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] Completion: Improve _man (3)
  2018-06-15 13:59           ` [PATCH] Completion: Improve _man (3) dana
@ 2018-06-15 14:05             ` Daniel Shahaf
  2018-06-15 14:21               ` dana
  0 siblings, 1 reply; 15+ messages in thread
From: Daniel Shahaf @ 2018-06-15 14:05 UTC (permalink / raw)
  To: dana; +Cc: Zsh workers

dana wrote on Fri, 15 Jun 2018 08:59 -0500:
> Round 3, i suppose:
> 
> * Re-add the partial section-name matching functionality that Daniel mentioned

Thanks for the quick turnaround.  With this patch, `man 3 utf<TAB>`,
`man 3per utf<TAB>`, and `man 3perll utf<TAB>` all complete to
`... utf8`.  That's correct for the first two but wrong for the last one:

%  man 3perll utf8
No manual entry for utf8 in section 3perll

Cheers,

Daniel

> * More accurately determine whether an initial operand could be a section name
> * Support .z compression extension (per man-db)
> * Add some more documentation
> 
> Daniel (or anyone), if you have time, could you see if this works as you'd
> expect?
> 
> dana


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] Completion: Improve _man (3)
  2018-06-15 14:05             ` Daniel Shahaf
@ 2018-06-15 14:21               ` dana
  2018-06-15 14:39                 ` Mikael Magnusson
  2018-06-15 14:47                 ` Daniel Shahaf
  0 siblings, 2 replies; 15+ messages in thread
From: dana @ 2018-06-15 14:21 UTC (permalink / raw)
  To: Daniel Shahaf; +Cc: Zsh workers

On 15 Jun 2018, at 09:05, Daniel Shahaf <d.s@daniel.shahaf.name> wrote:
>Thanks for the quick turnaround.  With this patch, `man 3 utf<TAB>`,
>`man 3per utf<TAB>`, and `man 3perll utf<TAB>` all complete to
>`... utf8`.  That's correct for the first two but wrong for the last one:

Oops, i confused myself looking at the old one. Fixing that actually improves
the partial matching a lot in general, thanks. Try this, if you don't mind.

dana


diff --git a/Completion/Unix/Command/_man b/Completion/Unix/Command/_man
index 11c2fab7f..ab283789b 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,23 @@ _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 {
+      if [[ $OSTYPE == solaris* || $1 != <->* ]]; then
+        dirs+=( $^_manpath/(sman|man|cat)$1(|.*)/ )
+      else
+        dirs+=( $^_manpath/(sman|man|cat)${1%%[^0-9]#}*/ )
+      fi
+    } $sects
+
     sect=${(j<|>)sects}
     [[ $sect == *'|'* ]] && sect="($sect)"
     awk="\$2 == \"$sect\" {print \$1}"
@@ -281,7 +310,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 +409,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[@]"


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] Completion: Improve _man (3)
  2018-06-15 14:21               ` dana
@ 2018-06-15 14:39                 ` Mikael Magnusson
  2018-06-15 14:55                   ` Daniel Shahaf
  2018-06-15 14:47                 ` Daniel Shahaf
  1 sibling, 1 reply; 15+ messages in thread
From: Mikael Magnusson @ 2018-06-15 14:39 UTC (permalink / raw)
  To: dana; +Cc: Daniel Shahaf, Zsh workers

On Fri, Jun 15, 2018 at 4:21 PM, dana <dana@dana.is> wrote:
> On 15 Jun 2018, at 09:05, Daniel Shahaf <d.s@daniel.shahaf.name> wrote:
>>Thanks for the quick turnaround.  With this patch, `man 3 utf<TAB>`,
>>`man 3per utf<TAB>`, and `man 3perll utf<TAB>` all complete to
>>`... utf8`.  That's correct for the first two but wrong for the last one:
>
> Oops, i confused myself looking at the old one. Fixing that actually improves
> the partial matching a lot in general, thanks. Try this, if you don't mind.
>
> @@ -232,7 +245,23 @@ _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 {
> +      if [[ $OSTYPE == solaris* || $1 != <->* ]]; then
> +        dirs+=( $^_manpath/(sman|man|cat)$1(|.*)/ )
> +      else
> +        dirs+=( $^_manpath/(sman|man|cat)${1%%[^0-9]#}*/ )
> +      fi
> +    } $sects
> +

Please don't use () for 1 {} a b c style loops in actual scripts, it
is not guaranteed to work. Instead, use () { for 1 { } } a b c which
is the documented syntax.

-- 
Mikael Magnusson


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] Completion: Improve _man (3)
  2018-06-15 14:21               ` dana
  2018-06-15 14:39                 ` Mikael Magnusson
@ 2018-06-15 14:47                 ` Daniel Shahaf
  1 sibling, 0 replies; 15+ messages in thread
From: Daniel Shahaf @ 2018-06-15 14:47 UTC (permalink / raw)
  To: dana; +Cc: Zsh workers

dana wrote on Fri, Jun 15, 2018 at 09:21:07 -0500:
> On 15 Jun 2018, at 09:05, Daniel Shahaf <d.s@daniel.shahaf.name> wrote:
> >Thanks for the quick turnaround.  With this patch, `man 3 utf<TAB>`,
> >`man 3per utf<TAB>`, and `man 3perll utf<TAB>` all complete to
> >`... utf8`.  That's correct for the first two but wrong for the last one:
> 
> Oops, i confused myself looking at the old one. Fixing that actually improves
> the partial matching a lot in general, thanks. Try this, if you don't mind.

Much better, thanks.

While here, I noticed this:

[[[
% zstyle \* group-name '' 
% zstyle \* format '> %d' 
% zstyle \* separate-sections true 
% man ut
> manual page, section 2 (system calls)
utime             utimensat
> manual page, section 3 (library functions)
utf8       utime
> manual page, section 7 (miscellanea)
utf-8        utf8         utime.h      utmpx.h
]]]

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.)

Cheers,

Daniel


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] Completion: Improve _man (3)
  2018-06-15 14:39                 ` Mikael Magnusson
@ 2018-06-15 14:55                   ` Daniel Shahaf
  2018-06-15 15:14                     ` dana
  2018-06-15 17:27                     ` Mikael Magnusson
  0 siblings, 2 replies; 15+ messages in thread
From: Daniel Shahaf @ 2018-06-15 14:55 UTC (permalink / raw)
  To: Mikael Magnusson; +Cc: dana, Zsh workers

Mikael Magnusson wrote on Fri, Jun 15, 2018 at 16:39:36 +0200:
> Please don't use () for 1 {} a b c style loops in actual scripts, it
> is not guaranteed to work. Instead, use () { for 1 { } } a b c which
> is the documented syntax.

Now that you mention it, I think completion functions may not use shortloops
syntax: compinit doesn't reset that option.

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.

Cheers,

Daniel


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] Completion: Improve _man (3)
  2018-06-15 14:55                   ` Daniel Shahaf
@ 2018-06-15 15:14                     ` dana
  2018-06-15 15:36                       ` Peter Stephenson
  2018-06-15 17:27                     ` Mikael Magnusson
  1 sibling, 1 reply; 15+ messages in thread
From: dana @ 2018-06-15 15:14 UTC (permalink / raw)
  To: Daniel Shahaf; +Cc: Mikael Magnusson, Zsh workers

On 15 Jun 2018, at 09:39, Mikael Magnusson <mikachu@gmail.com> 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@daniel.shahaf.name> 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@daniel.shahaf.name> 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@daniel.shahaf.name> 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))


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] Completion: Improve _man (3)
  2018-06-15 15:14                     ` dana
@ 2018-06-15 15:36                       ` Peter Stephenson
  0 siblings, 0 replies; 15+ messages in thread
From: Peter Stephenson @ 2018-06-15 15:36 UTC (permalink / raw)
  To: Zsh workers

On Fri, 15 Jun 2018 10:14:20 -0500
dana <dana@dana.is> wrote:
> On 15 Jun 2018, at 09:39, Mikael Magnusson <mikachu@gmail.com> 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 :(

It is certainly a long-standing general expectation that variant syntax
is avoided in widely distributed shell code as it's both a confusion and
a possible source of problems with people's own option settings  ---
completion deliberately doesn't do full emulation, just sanitize
important options locally.

I don't think this is currently explicitly mentioned in
Etc/completion-style-guide, though...

pws


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] Completion: Improve _man (3)
  2018-06-15 14:55                   ` Daniel Shahaf
  2018-06-15 15:14                     ` dana
@ 2018-06-15 17:27                     ` Mikael Magnusson
  1 sibling, 0 replies; 15+ messages in thread
From: Mikael Magnusson @ 2018-06-15 17:27 UTC (permalink / raw)
  To: Daniel Shahaf; +Cc: dana, Zsh workers

On Fri, Jun 15, 2018 at 4:55 PM, Daniel Shahaf <d.s@daniel.shahaf.name> wrote:
> Mikael Magnusson wrote on Fri, Jun 15, 2018 at 16:39:36 +0200:
>> Please don't use () for 1 {} a b c style loops in actual scripts, it
>> is not guaranteed to work. Instead, use () { for 1 { } } a b c which
>> is the documented syntax.
>
> Now that you mention it, I think completion functions may not use shortloops
> syntax: compinit doesn't reset that option.
>
> 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.

It isn't shortloops syntax, but it exploits the fact that writing a
for loop with braces also tells the anonymous function parser that the
function definition ends while not ending the command itself. Leaving
out the 'in a b c' or '(a b c)' part is documented as using the
positional arguments, and works in dash as well. Using 1 as the loop
variable doesn't work in dash, and is possibly somewhat questionable,
but I don't see why we would ever break that, and it saves a 'local'.

Dana wrote:
> Someone on IRC told me it was safe :(

It probably is, but if someone ever decides to rewrite the parser, it
might be hard to retain this particular quirk. When you asked, you
didn't say it was for a to-be distributed completion script :).

Peter wrote:
> It is certainly a long-standing general expectation that variant syntax
> is avoided in widely distributed shell code as it's both a confusion and
> a possible source of problems with people's own option settings

Is it actually possible to disable the alternative syntax? In any
case, it might be nicer to do
() { for 1; do ..stuff..; done } $sects
or even
() { local sect; for sect; do ..stuff..; done } $sects
or even even
() { local sect; for sect in $sects; do ..stuff..; done }

-- 
Mikael Magnusson


^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2018-06-15 17:28 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-06-10  6:06 [PATCH] Completion: Improve _man dana
2018-06-10 13:07 ` Oliver Kiddle
2018-06-10 14:02   ` dana
2018-06-11 10:48     ` [PATCH] Completion: Improve _man (2) dana
2018-06-14  9:50       ` Daniel Shahaf
2018-06-14 10:20         ` dana
2018-06-15 13:59           ` [PATCH] Completion: Improve _man (3) dana
2018-06-15 14:05             ` Daniel Shahaf
2018-06-15 14:21               ` dana
2018-06-15 14:39                 ` Mikael Magnusson
2018-06-15 14:55                   ` Daniel Shahaf
2018-06-15 15:14                     ` dana
2018-06-15 15:36                       ` Peter Stephenson
2018-06-15 17:27                     ` Mikael Magnusson
2018-06-15 14:47                 ` Daniel Shahaf

Code repositories for project(s) associated with this public inbox

	https://git.vuxu.org/mirror/zsh/

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).