Completion for man (_man) patch to support -M

Completion for man (_man) patch to support -M

[Silas Silva]

[-- Attachment #1: Type: text/plain, Size: 1310 bytes --]

Hi all,

I was studying the completion system and, since I would like to add
support for -M for the _man completion, I've done it.  Dirty and ugly
patch is attached.

Anyway, here comes some inline considerations:


    +  if (( $words[(I)-M] == (( $CURRENT - 1 )) )); then
    +    _directories && return 0
    +  fi
    +

Here I wanted to check if the cursor position (where the user will hit
tab) will be just after the -M option.  If true, it complete for
directories and exit.  Is it right?  Is there any better way to make
that check?

First I tried to use the _arguments function, but I realized that
_arguments is just an easy layer for what happens behind the scenes and
_man (and other completion functions) use the "behind the scenes" way to
make things work.


    +  if (( $words[(I)-M] )); then
    +    local opt
    +    opt=$words[(( $words[(I)-M]+1 ))]
    +    _manpath=($_manpath $opt)
    +  fi
    +

_man sets a local MANPATH to make man look for man pages in the right
places.  If the user set a man path with -M, I want it to be added to
the _manpath variable.

I'm just not sure if it is the better way to do it.  opt holds the
option passed to the -M flag, but (( $words[(I)-M]+1 )) looks ugly?

If it is all right, can it be pulled upstream?

Thank you very much!

-- 
Silas Silva

[-- Attachment #2: man--M.diff --]
[-- Type: text/plain, Size: 717 bytes --]

--- _man.orig	2010-10-21 12:48:02.000000000 -0200
+++ _man	2010-10-21 12:44:37.000000000 -0200
@@ -3,6 +3,10 @@
 _man() {
   local dirs expl mrd awk
 
+  if (( $words[(I)-M] == (( $CURRENT - 1 )) )); then
+    _directories && return 0
+  fi
+
   if [[ $service == man ]] && (( $words[(I)-l] + $words[(I)--local-file] )); then
     _files || return 0
   fi
@@ -21,6 +25,12 @@
   (( $#_manpath )) ||
       _manpath=( /usr/man(-/) /(opt|usr)/(pkg|dt|share|X11R6|local)/(cat|)man(-/) )
 
+  if (( $words[(I)-M] )); then
+    local opt
+    opt=$words[(( $words[(I)-M]+1 ))]
+    _manpath=($_manpath $opt)
+  fi
+
   # `sman' is the SGML manual directory for Solaris 7.
   # 1M is system administrator commands on SVR4
 

Re: Completion for man (_man) patch to support -M

[Peter Stephenson]

On Thu, 21 Oct 2010 13:15:14 -0200
Silas Silva <silasdb@gmail.com> wrote:
> I was studying the completion system and, since I would like to add
> support for -M for the _man completion, I've done it.  Dirty and ugly
> patch is attached.

Not sure how widespread support for that argument is, certainly man is
historically rather different on different systems, but _man already
doesn't look much like other completion functions, and the intention
here is obvious enough, so I've committed it.

(For years I've wanted to make man complete files, i.e. by direct path
to the file to be read rather than via manual path + suffix, when
supported, e.g. by GNU, but never unpicked _man enough to do it.)

>     +  if (( $words[(I)-M] == (( $CURRENT - 1 )) )); then
>     +    _directories && return 0
>     +  fi
>     +
> 
> Here I wanted to check if the cursor position (where the user will hit
> tab) will be just after the -M option.  If true, it complete for
> directories and exit.  Is it right?  Is there any better way to make
> that check?

That will do fine.  There are probably other ways of doing it, but
CURRENT and words are the right variables.

>     +  if (( $words[(I)-M] )); then
>     +    local opt
>     +    opt=$words[(( $words[(I)-M]+1 ))]
>     +    _manpath=($_manpath $opt)
>     +  fi
>     +

There's actually a philosphical bug there.  You need $(( ... )) to return the
expanded value, not (( ... )).  However, I say it's a philosophical bug
because actually the contents of the [...] are evaluated as an
arithmetical expression anyway, so you can omit the parentheses --- and
when they're there, the simply do grouping as in any other arithmetic
expression, so actually what you've got works fine, slightly accidentally.

> _man sets a local MANPATH to make man look for man pages in the right
> places.  If the user set a man path with -M, I want it to be added to
> the _manpath variable.
> 
> I'm just not sure if it is the better way to do it.  opt holds the
> option passed to the -M flag, but (( $words[(I)-M]+1 )) looks ugly?

Well, apart from the obvious use of a variable (and omitting the
parentheses),

  integer ind=$words[(I)-M]
  if (( ind )); then
    local opt
    opt=$words[ind+1]
    _manpath=($_manpath $opt)
  fi

I don't see a major improvement.   (I'll commit this improvement on top.)

-- 
Peter Stephenson <p.w.stephenson@ntlworld.com>
Web page now at http://homepage.ntlworld.com/p.w.stephenson/

Re: Completion for man (_man) patch to support -M

[Silas Silva]

On Thu, Oct 21, 2010 at 08:29:12PM +0100, Peter Stephenson wrote:
> On Thu, 21 Oct 2010 13:15:14 -0200
> Silas Silva <silasdb@gmail.com> wrote:
> > I was studying the completion system and, since I would like to add
> > support for -M for the _man completion, I've done it.  Dirty and ugly
> > patch is attached.
> 
> Not sure how widespread support for that argument is, certainly man is
> historically rather different on different systems, but _man already
> doesn't look much like other completion functions, and the intention
> here is obvious enough, so I've committed it.

Yeah...  I tested it under NetBSD and GNU/Linux.  It might work under
other BSDs as well, but I don't know about other Unices.

> > I'm just not sure if it is the better way to do it.  opt holds the
> > option passed to the -M flag, but (( $words[(I)-M]+1 )) looks ugly?
> 
> Well, apart from the obvious use of a variable (and omitting the
> parentheses),
> 
>   integer ind=$words[(I)-M]
>   if (( ind )); then
>     local opt
>     opt=$words[ind+1]
>     _manpath=($_manpath $opt)
>   fi
> 
> I don't see a major improvement.   (I'll commit this improvement on top.)

Yeah, having the obvious use of a variable is a good practice.  Thanks
for the lesson and for the commit.   :-)

-- 
Silas Silva

Re: Completion for man (_man) patch to support -M

[Greg Klanderman]

>>>>> On October 21, 2010 Peter Stephenson <p.w.stephenson@ntlworld.com> wrote:

> (For years I've wanted to make man complete files, i.e. by direct path
> to the file to be read rather than via manual path + suffix, when
> supported, e.g. by GNU, but never unpicked _man enough to do it.)

Yeah, I've wanted that for a while; the separate-sections zstyle
someone posted recently has definitely been an improvement, but it'd
be great to actually be able to pick amongst the files, especially now
that on debian at least there are these sub-extensions meaning you can
have multiple pages of the same name in a given section.

Greg