Fix manpath handling in create_manpage_completions.py #6879
Conversation
...as well as do some (very!) light cleanup. Currently, `create_manpage_completions.py` does not properly understand/respect the `$MANPATH` variable. One important feature of `$MANPATH` is that an empty component (i.e. the trailing : in `foo:bar:`) expands to the 'default' or 'system' path -- that is to say, the path that would be used if `$MANPATH` was unset. This allows the user to extend the manpath without clobbering it, and has been a feature many Unices have included for years. The current implementation blindly uses the `$MANPATH` variable if it exists, which does not allow for this behaviour -- to expand the variable correctly, an external program must be invoked. Therefore, we first shell out to the 'proper' (read: best guess) external program. If that fails, we can then try to use `$MANPATH` directly/literally. Finally, if both of those are impossible, we can fall back to some common paths from widely used operating systems. Note that the `man.conf` parsing has been removed: this is because while many 'traditional' Unices (BSDs, Solaris, macOS) support this file, only macOS actually ships a file -- most other Unices use a `conf.d`-style layout and supporting that from our Python is impractical and silly at best. On GNU (read: Linux) systems, `mandb` uses `/etc/man_db.conf` with slightly different syntax and sematics. As this code-path has bitrotted (and likely never worked, anyway), just remove it. `create_manpage_completions.py` looks like it has suffered a lot of confusion and bitrot in general over the last few years -- and is overdue for a major refactoring. I am quite interested in tackling this, but I plan to wait until the go-ahead to drop support for Python 2 is given, as a major refactor/rewrite that still supports Python 2 (and thus ignores the ergonomic/API/syntax improvements of Python 3) does not make sense to me. Related: #5657 It would probably be good to revisit `man.fish` once again when a comprehensive refactor happens: hopefully every permutation of `man`/`$MANPATH` could be documented as part of that effort.
The comments tell you where we read this file! For mandoc, typically used on OpenBSD, but also on some linuxen. Please put it back. |
My bad -- didn't realize that codepath was actually reachable since a quick read seemed to indicate it would throw an exception. I've re-implemented the functionality with regex (a lot more readable)... Let me know if this will work for OpenBSD -- I don't have any |
I was not aware that this codepath was used -- since it appeared that it would throw an error when it was reached. Redo it, using regex, and support parsing NetBSD man.conf as well (untested).
| @@ -975,6 +974,16 @@ def get_paths_from_man_locations(): | |||
| # If we can't have the OS interpret $MANPATH, just use it as-is (gulp). | |||
| if not parent_paths and os.getenv("MANPATH"): | |||
| parent_paths = os.getenv("MANPATH").strip().split(":") | |||
| # Fallback: With mandoc (OpenBSD, embedded Linux) and NetBSD man, the only way to get the default manpath is by reading /etc. | |||
There was a problem hiding this comment.
No, NetBSD man has man -p, as the comment above stated.
Please restore that.
There was a problem hiding this comment.
NetBSD's man -p actually prints the section directories instead of the manpath directories -- that was my comment that I added, and it was in slight error. We could special-case man -p to take the dirname of each entry -- or we could parse the man.conf file as in https://netbsd.gw.com/cgi-bin/man-cgi?man.conf+5+NetBSD-current
|
|
||
| # Most (GNU, macOS, Haiku) modern implementations of man support being called with `--path`. | ||
| # Traditional implementations require a second `manpath` program: examples include FreeBSD and Solaris. | ||
| # Prefer an external program first because these programs return a superset of the $MANPATH variable. |
There was a problem hiding this comment.
This prefers the external program by calling both, and only keeping the latter output?
There was a problem hiding this comment.
Looks like I missed a break here... It should go on line 979.
|
Merged, thanks and sorry for the delay! |
|
Added the extra |
...as well as do some (very!) light cleanup.
Currently,
create_manpage_completions.pydoes not properlyunderstand/respect the
$MANPATHvariable. One important feature of$MANPATHis that an empty component (i.e. the trailing : infoo:bar:) expands to the 'default' or 'system' path -- that is to say,the path that would be used if
$MANPATHwas unset. This allows theuser to extend the manpath without clobbering it, and has been a feature
many Unices have included for years.
The current implementation blindly uses the
$MANPATHvariable if itexists, which does not allow for this behaviour -- to expand the
variable correctly, an external program must be invoked. Therefore, we
first shell out to the 'proper' (read: best guess) external program. If
that fails, we can then try to use
$MANPATHdirectly/literally.Finally, if both of those are impossible, we can fall back to some
common paths from widely used operating systems.
Note that the
man.confparsing has been removed: this is because whilemany 'traditional' Unices (BSDs, Solaris, macOS) support this file, only
macOS actually ships a file -- most other Unices use a
conf.d-stylelayout and supporting that from our Python is impractical and silly at
best. On GNU (read: Linux) systems,
mandbuses/etc/man_db.confwithslightly different syntax and sematics. As this code-path has bitrotted
(and likely never worked, anyway), just remove it.
create_manpage_completions.pylooks like it has suffered a lot ofconfusion and bitrot in general over the last few years -- and is
overdue for a major refactoring. I am quite interested in tackling this,
but I plan to wait until the go-ahead to drop support for Python 2 is
given, as a major refactor/rewrite that still supports Python 2 (and
thus ignores the ergonomic/API/syntax improvements of Python 3) does not
make sense to me.
Related: #5657
It would probably be good to revisit
man.fishonce again when acomprehensive refactor happens: hopefully every permutation of
man/$MANPATHcould be documented as part of that effort.