_man completion update for Solaris

_man completion update for Solaris

[Danek Duvall]

After finally looking at Daniel's _man changes, I saw that there were a
couple of small (pre-existing) issues on Solaris.  Namely: the -s option
can take a comma-separated list of sections; and the man-index directory
gets thrown into the mix when it shouldn't.  This patch takes care of those
two things.

I don't know if the comma/colon section separation should just be generic
-- it's safe to do on Solaris, since although we don't recognize the
separator, we don't have any sections that have colons in them -- but I
don't know if it's compatible with the third if clause there.

I also didn't protect the /man-index/ removal with an OSTYPE check because
that seems safe enough on any OS, but there may be no reason not to do
that, either.

Thanks,
Danek

diff --git a/Completion/Unix/Command/_man b/Completion/Unix/Command/_man
index ae6ac38..b2aaeaf 100644
--- a/Completion/Unix/Command/_man
+++ b/Completion/Unix/Command/_man
@@ -46,6 +46,7 @@ _man() {
   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//,/|}"
@@ -67,6 +68,8 @@ _man() {
     dirs=( $^_manpath/(sman|man|cat)*/ )
     awk='{print $1}'
   fi
+  # 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)}%/} )

Re: _man completion update for Solaris

[Daniel Shahaf]

Danek Duvall wrote on Tue, Sep 06, 2016 at 11:06:14 -0700:
> After finally looking at Daniel's _man changes, I saw that there were a
> couple of small (pre-existing) issues on Solaris.

Thanks for the review.

> Namely: the -s option can take a comma-separated list of sections; and
> the man-index directory gets thrown into the mix when it shouldn't.
> This patch takes care of those two things.

Thanks.  The patch looks good to me.  I'll push it tomorrow (not right
now to give others a chance to respond).

> I don't know if the comma/colon section separation should just be generic
> -- it's safe to do on Solaris, since although we don't recognize the
> separator, we don't have any sections that have colons in them -- but I
> don't know if it's compatible with the third if clause there.

"Third if clause" refers to what?  If you mean the 'else' branch just
below the context of the first hunk, then on my system «man 7,8 foo»
looks for a section literally called "man7,8" whereas the same with -S
looks in the "man7" and "man8" sections, so at least on my system the
'else" branch shouldn't s/,/|/.  (My system runs Debian stable.)

> I also didn't protect the /man-index/ removal with an OSTYPE check because
> that seems safe enough on any OS, but there may be no reason not to do
> that, either.

Sure.

Thanks for the patch,

Daniel

Re: _man completion update for Solaris

[Danek Duvall]

On Tue, Sep 06, 2016 at 10:45:08PM +0000, Daniel Shahaf wrote:

> Danek Duvall wrote on Tue, Sep 06, 2016 at 11:06:14 -0700:
> > After finally looking at Daniel's _man changes, I saw that there were a
> > couple of small (pre-existing) issues on Solaris.
> 
> Thanks for the review.
> 
> > Namely: the -s option can take a comma-separated list of sections; and
> > the man-index directory gets thrown into the mix when it shouldn't.
> > This patch takes care of those two things.
> 
> Thanks.  The patch looks good to me.  I'll push it tomorrow (not right
> now to give others a chance to respond).

Great, thanks.

> > I don't know if the comma/colon section separation should just be generic
> > -- it's safe to do on Solaris, since although we don't recognize the
> > separator, we don't have any sections that have colons in them -- but I
> > don't know if it's compatible with the third if clause there.
> 
> "Third if clause" refers to what?  If you mean the 'else' branch just
> below the context of the first hunk, then on my system «man 7,8 foo»
> looks for a section literally called "man7,8" whereas the same with -S
> looks in the "man7" and "man8" sections, so at least on my system the
> 'else" branch shouldn't s/,/|/.  (My system runs Debian stable.)

"elif (( CURRENT > 2 ))", but the behavior you describe is the one I was
wondering about.

Thanks,
Danek