zsh-workers
 help / color / mirror / code / Atom feed
* man completion
@ 2022-05-15 15:20 Karel Balej
  2022-05-15 16:45 ` Bart Schaefer
  0 siblings, 1 reply; 10+ messages in thread
From: Karel Balej @ 2022-05-15 15:20 UTC (permalink / raw)
  To: zsh-workers

Hello,

per the `man` manual page:

> If MANPATH begins with a colon, it is appended to the standard path; if
> it ends with a colon, it is prepended to the standard path; or if it
> contains two adjacent colons, the standard path is inserted between the
> colons.

Yet when I export MANPATH as ":/some/path", I only get suggestions from
this directory and not from the default directories. Could this detail
be overlooked in the completion function?

Kind regards,
Karel


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

* Re: man completion
  2022-05-15 15:20 man completion Karel Balej
@ 2022-05-15 16:45 ` Bart Schaefer
  2022-05-16 19:41   ` Bart Schaefer
  0 siblings, 1 reply; 10+ messages in thread
From: Bart Schaefer @ 2022-05-15 16:45 UTC (permalink / raw)
  To: Karel Balej; +Cc: Zsh hackers list

On Sun, May 15, 2022 at 8:20 AM Karel Balej <karelb@gimli.ms.mff.cuni.cz> wrote:
>
> Yet when I export MANPATH as ":/some/path", I only get suggestions from
> this directory and not from the default directories. Could this detail
> be overlooked in the completion function?

The completion function is calling $(manpath 2>/dev/null), but it's
caching the result, so if you change $MANPATH that's not being picked
up.

It also clobbers the cache if called with the -M or -m options and
doesn't reset it, which is clearly wrong.

A quick but suboptimal fix (I will not be pushing this to git):

diff --git a/Completion/Unix/Command/_man b/Completion/Unix/Command/_man
index dba1d13dc..0e0415cc7 100644
--- a/Completion/Unix/Command/_man
+++ b/Completion/Unix/Command/_man
@@ -16,7 +16,7 @@
 _man() {
   local dirs expl mrd awk variant noinsert
   local -a context line state state_descr args modes
-  local -aU sects
+  local -aU sects _manpath
   local -A opt_args val_args sect_descs

   if [[ $service == man ]]; then


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

* Re: man completion
  2022-05-15 16:45 ` Bart Schaefer
@ 2022-05-16 19:41   ` Bart Schaefer
  2022-05-16 21:40     ` Mikael Magnusson
  2022-05-17  0:41     ` Bart Schaefer
  0 siblings, 2 replies; 10+ messages in thread
From: Bart Schaefer @ 2022-05-16 19:41 UTC (permalink / raw)
  To: Karel Balej; +Cc: Zsh hackers list

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

On Sun, May 15, 2022 at 9:45 AM Bart Schaefer <schaefer@brasslantern.com> wrote:
>
> The completion function is calling $(manpath 2>/dev/null), but it's
> caching the result, so if you change $MANPATH that's not being picked
> up.
>
> It also clobbers the cache if called with the -M or -m options and
> doesn't reset it, which is clearly wrong.

Here's a patch that attempts to (1) skip the cache entirely if the -M
option appears and (2) assure a cache miss if $MANPATH has changed.

The question is whether it's worth this much effort to avoid running
$(manpath 2>/dev/null) every time.

Also ... I didn't change this part, but doing ${(s.:.)...} twice seems
redundant or even potentially wrong?

+      mp=( ${(s.:.)$(manpath 2>/dev/null)} )
+      [[ "$mp" == *:* ]] && mp=( ${(s.:.)mp} )

[-- Attachment #2: _man.txt --]
[-- Type: text/plain, Size: 1922 bytes --]

diff --git a/Completion/Unix/Command/_man b/Completion/Unix/Command/_man
index dba1d13dc..22fd348fa 100644
--- a/Completion/Unix/Command/_man
+++ b/Completion/Unix/Command/_man
@@ -16,7 +16,7 @@
 _man() {
   local dirs expl mrd awk variant noinsert
   local -a context line state state_descr args modes
-  local -aU sects
+  local -aU sects _manpath
   local -A opt_args val_args sect_descs
 
   if [[ $service == man ]]; then
@@ -168,29 +168,36 @@ _man() {
   _arguments -s -S : $args '*::: :->man' && return 0
   [[ -n $state ]] || return 1
 
-  if (( ! $#_manpath )); then
-    local mp
-    mp=( ${(s.:.)$(manpath 2>/dev/null)} )
-    [[ "$mp" == *:* ]] && mp=( ${(s.:.)mp} )
-    if (( $#mp )); then
-      _manpath=( $mp )
-    elif (( $#manpath )); then
-      _manpath=( $manpath )
-    fi
-  fi
-
-  (( $#_manpath )) ||
-      _manpath=( /usr/man(-/) /(opt|usr)/(pkg|dt|share|X11R6|local)/(cat|)man(-/) )
-
   # Override man path
   [[ -n ${opt_args[-M]} ]] &&
   _manpath=( ${(s<:>)opt_args[-M]} )
 
+  # Restore cached man path to avoid $(manpath) if we can
+  if (( ! $#_manpath )); then
+    if (( ! $+_manpath_cache )); then
+      typeset -gHA _manpath_cache
+    fi
+    if [[ -z $_manpath_cache[$MANPATH] ]]; then
+      local mp
+      mp=( ${(s.:.)$(manpath 2>/dev/null)} )
+      [[ "$mp" == *:* ]] && mp=( ${(s.:.)mp} )
+      if (( $#mp )); then
+        _manpath_cache[$MANPATH]=${(j.:.)mp}
+      elif (( $#manpath )); then
+        _manpath_cache[$MANPATH]=$MANPATH
+      fi
+    fi
+    _manpath=( ${(s.:.)_manpath_cache[$MANPATH]} )
+  fi
+
   # Augment man path
   [[ $variant == (netbsd|openbsd)* ]] &&
   [[ -n ${opt_args[-m]} ]] &&
   _manpath+=( ${(s<:>)opt_args[-m]} )
 
+  (( $#_manpath )) ||
+      _manpath=( /usr/man(-/) /(opt|usr)/(pkg|dt|share|X11R6|local)/(cat|)man(-/) )
+
   # `sman' is the SGML manual directory for Solaris 7.
   # 1M is system administrator commands on SVR4
 

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

* Re: man completion
  2022-05-16 19:41   ` Bart Schaefer
@ 2022-05-16 21:40     ` Mikael Magnusson
  2022-05-17  0:41     ` Bart Schaefer
  1 sibling, 0 replies; 10+ messages in thread
From: Mikael Magnusson @ 2022-05-16 21:40 UTC (permalink / raw)
  To: Bart Schaefer; +Cc: Karel Balej, Zsh hackers list

On 5/16/22, Bart Schaefer <schaefer@brasslantern.com> wrote:
> On Sun, May 15, 2022 at 9:45 AM Bart Schaefer <schaefer@brasslantern.com>
> wrote:
>>
>> The completion function is calling $(manpath 2>/dev/null), but it's
>> caching the result, so if you change $MANPATH that's not being picked
>> up.
>>
>> It also clobbers the cache if called with the -M or -m options and
>> doesn't reset it, which is clearly wrong.
>
> Here's a patch that attempts to (1) skip the cache entirely if the -M
> option appears and (2) assure a cache miss if $MANPATH has changed.
>
> The question is whether it's worth this much effort to avoid running
> $(manpath 2>/dev/null) every time.
>
> Also ... I didn't change this part, but doing ${(s.:.)...} twice seems
> redundant or even potentially wrong?
>
> +      mp=( ${(s.:.)$(manpath 2>/dev/null)} )
> +      [[ "$mp" == *:* ]] && mp=( ${(s.:.)mp} )

As far as I'm aware, it should be impossible for this condition to be
true since (s.:.) on the first line will have removed all colons
already, unless you somehow have IFS set to : without breaking
anything else.

-- 
Mikael Magnusson


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

* Re: man completion
  2022-05-16 19:41   ` Bart Schaefer
  2022-05-16 21:40     ` Mikael Magnusson
@ 2022-05-17  0:41     ` Bart Schaefer
  2022-05-21 12:57       ` Karel Balej
  1 sibling, 1 reply; 10+ messages in thread
From: Bart Schaefer @ 2022-05-17  0:41 UTC (permalink / raw)
  To: Karel Balej; +Cc: Zsh hackers list

On Mon, May 16, 2022 at 12:41 PM Bart Schaefer
<schaefer@brasslantern.com> wrote:
>
> Here's a patch that attempts to (1) skip the cache entirely if the -M
> option appears

Hm, my "man man" says of -M
> ... overrides the $MANPATH environment variable and causes option -m to be ignored.

Whereas the AIX help string in _man says
> '-m[only search paths specified by -M/MANPATH]'

And of course on linux/macos/dragonfly/freebsd -m means to search
manuals of other operating systems, whereas on netbsd/openbsd it means
to append to MANPATH.

The last of those seems to be the only alternative actually supported.

First question, if the argument to -M has leading/doubled/trailing
colons, is it supposed to be treated like $MANPATH with respect to
adding the system paths?  (It doesn't appear so, on Ubuntu 20 at
least).

Second, does anyone know where the manuals of other OSs would be found?

Third, what additional locations should be searched on AIX when -m is
not present?  (And does anyone care?)


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

* Re: man completion
  2022-05-17  0:41     ` Bart Schaefer
@ 2022-05-21 12:57       ` Karel Balej
  2022-05-21 20:22         ` Bart Schaefer
  0 siblings, 1 reply; 10+ messages in thread
From: Karel Balej @ 2022-05-21 12:57 UTC (permalink / raw)
  To: Bart Schaefer; +Cc: Zsh hackers list

Thank you for looking into this.

Inspired by your messages, I have been able to solve my problem. Just to
clarify, my issue was not that zsh would not pick up changes made to
MANPATH after the shell is started. I set MANPATH in /etc/profile which
is read by zsh and I was confused that it was not being appended to the
standard paths even though it started with a colon. Turns out the
problem was that I was using mandoc instead of man-db as the man
provider, which however does not provide the manpath command.

This is probably still a bug though, as the comment in _man claims that
the function assumes either man-db or mandoc is used on Linux.

> First question, if the argument to -M has leading/doubled/trailing
> colons, is it supposed to be treated like $MANPATH with respect to
> adding the system paths?  (It doesn't appear so, on Ubuntu 20 at
> least).

The manual page does not mention that it should be combined in any way -
it only says _override_.

> Third, what additional locations should be searched on AIX when -m is
> not present?  (And does anyone care?)

Judging by the manual [1], probably just the /usr/share/man directory
structure.

[1] https://www.ibm.com/docs/en/aix/7.2?topic=m-man-command

Regards,
Karel


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

* Re: man completion
  2022-05-21 12:57       ` Karel Balej
@ 2022-05-21 20:22         ` Bart Schaefer
  2022-05-22 16:00           ` Karel Balej
  0 siblings, 1 reply; 10+ messages in thread
From: Bart Schaefer @ 2022-05-21 20:22 UTC (permalink / raw)
  To: Karel Balej; +Cc: Zsh hackers list

On Sat, May 21, 2022 at 5:57 AM Karel Balej <karelb@gimli.ms.mff.cuni.cz> wrote:
>
> problem was that I was using mandoc instead of man-db as the man
> provider, which however does not provide the manpath command.
>
> This is probably still a bug though, as the comment in _man claims that
> the function assumes either man-db or mandoc is used on Linux.

That really applies only to the structure of the tree and files it
looks for within the path; we don't attempt to reproduce the
functionality of `manpath` itself.  Is there any manpath equivalent
for mandoc?

> > Third, what additional locations should be searched on AIX when -m is
> > not present?  (And does anyone care?)
>
> Judging by the manual [1], probably just the /usr/share/man directory
> structure.

Thanks.


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

* Re: man completion
  2022-05-21 20:22         ` Bart Schaefer
@ 2022-05-22 16:00           ` Karel Balej
  2022-05-22 21:26             ` Bart Schaefer
  0 siblings, 1 reply; 10+ messages in thread
From: Karel Balej @ 2022-05-22 16:00 UTC (permalink / raw)
  To: Bart Schaefer; +Cc: Zsh hackers list

> That really applies only to the structure of the tree and files it
> looks for within the path; we don't attempt to reproduce the
> functionality of `manpath` itself.  Is there any manpath equivalent
> for mandoc?

It seems that `man -w` does this for both man-db and mandoc (no mention
about it in the AIX man manual page for instance, though), so it might
be worth calling that instead of `manpath` on the relevant platforms so
that both the providers are supported.

K.


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

* Re: man completion
  2022-05-22 16:00           ` Karel Balej
@ 2022-05-22 21:26             ` Bart Schaefer
  2022-05-23 12:28               ` Karel Balej
  0 siblings, 1 reply; 10+ messages in thread
From: Bart Schaefer @ 2022-05-22 21:26 UTC (permalink / raw)
  To: Karel Balej; +Cc: Zsh hackers list

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

On Sun, May 22, 2022 at 8:59 AM Karel Balej <karelb@gimli.ms.mff.cuni.cz> wrote:
>
> It seems that `man -w` does this for both man-db and mandoc (no mention
> about it in the AIX man manual page for instance, though)

The existing completion function implies that -w / --path / --where
all work on aix and solaris.  There's no clear indication whether it
can be called with no arguments to display all possible paths ... but
let's try the following and see if anyone complains.  Includes the
previous patch.

[-- Attachment #2: _man.txt --]
[-- Type: text/plain, Size: 2151 bytes --]

diff --git a/Completion/Unix/Command/_man b/Completion/Unix/Command/_man
index dba1d13dc..190811e41 100644
--- a/Completion/Unix/Command/_man
+++ b/Completion/Unix/Command/_man
@@ -16,7 +16,7 @@
 _man() {
   local dirs expl mrd awk variant noinsert
   local -a context line state state_descr args modes
-  local -aU sects
+  local -aU sects _manpath
   local -A opt_args val_args sect_descs
 
   if [[ $service == man ]]; then
@@ -168,29 +168,40 @@ _man() {
   _arguments -s -S : $args '*::: :->man' && return 0
   [[ -n $state ]] || return 1
 
+  # Override man path
+  [[ -n ${opt_args[-M]} ]] &&
+  _manpath=( ${(s<:>)opt_args[-M]} )
+
+  # Restore cached man path to avoid $(manpath) if we can
   if (( ! $#_manpath )); then
-    local mp
-    mp=( ${(s.:.)$(manpath 2>/dev/null)} )
-    [[ "$mp" == *:* ]] && mp=( ${(s.:.)mp} )
-    if (( $#mp )); then
-      _manpath=( $mp )
-    elif (( $#manpath )); then
-      _manpath=( $manpath )
+    if (( ! $+_manpath_cache )); then
+      typeset -gHA _manpath_cache
     fi
+    if [[ -z $_manpath_cache[$MANPATH] ]]; then
+      local mp
+      mp=( ${(s.:.)$({ command man -w || manpath } 2>/dev/null)} )
+      [[ "$mp" == *:* ]] && mp=( ${(s.:.)mp} )
+      if (( $#mp )); then
+        _manpath_cache[$MANPATH]=${(j.:.)mp}
+      elif (( $#manpath )); then
+        _manpath_cache[$MANPATH]=$MANPATH
+      fi
+    fi
+    _manpath=( ${(s.:.)_manpath_cache[$MANPATH]} )
+  fi
+
+  # Augment man path
+  if [[ -n ${opt_args[-m]} ]]; then
+    [[ $variant == (netbsd|openbsd)* ]] &&
+    _manpath+=( ${(s<:>)opt_args[-m]} )
+  elif [[ $variant == aix* ]]; then
+    # _manpath declared -U so no need to test
+    _manpath+=( /usr/share/man )
   fi
 
   (( $#_manpath )) ||
       _manpath=( /usr/man(-/) /(opt|usr)/(pkg|dt|share|X11R6|local)/(cat|)man(-/) )
 
-  # 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
 

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

* Re: man completion
  2022-05-22 21:26             ` Bart Schaefer
@ 2022-05-23 12:28               ` Karel Balej
  0 siblings, 0 replies; 10+ messages in thread
From: Karel Balej @ 2022-05-23 12:28 UTC (permalink / raw)
  To: Bart Schaefer; +Cc: Zsh hackers list

> The existing completion function implies that -w / --path / --where
> all work on aix and solaris.  There's no clear indication whether it
> can be called with no arguments to display all possible paths ... but
> let's try the following and see if anyone complains.  Includes the
> previous patch.

This seems to work for me, both with mandoc and man-db. Thank you again
for investing time into this.

Best regards,
K.


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

end of thread, other threads:[~2022-05-23 12:28 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-15 15:20 man completion Karel Balej
2022-05-15 16:45 ` Bart Schaefer
2022-05-16 19:41   ` Bart Schaefer
2022-05-16 21:40     ` Mikael Magnusson
2022-05-17  0:41     ` Bart Schaefer
2022-05-21 12:57       ` Karel Balej
2022-05-21 20:22         ` Bart Schaefer
2022-05-22 16:00           ` Karel Balej
2022-05-22 21:26             ` Bart Schaefer
2022-05-23 12:28               ` Karel Balej

Code repositories for project(s) associated with this 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).