Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding directories to the MANPATH is hard. #2090

Closed
hugomg opened this issue May 24, 2015 · 27 comments
Closed

Adding directories to the MANPATH is hard. #2090

hugomg opened this issue May 24, 2015 · 27 comments

Comments

@hugomg
Copy link
Contributor

hugomg commented May 24, 2015

Fish treats the MANPATH environment specially and expands it into an array. This unfortunately makes it very easy to mess up the MANPATH and make all system manpages stop working.

The way MANPATH works is that like this:

  • If MANPATH is unset then man will search for man pages in some default system directories, as specified in /etc/manpath.config)
  • If MANPATH is a colon separated list of directories, man searches for manual pages in those directories and DOES NOT search in the default system directories

If you want to put some custom directories in the MANPATH while still having man use the system manpages then you must make sure that MANPATH either starts or ends with a colon or has a :: in the middle. From man manpath:

ENVIRONMENT
       MANPATH
              If $MANPATH is set,  manpath  displays  its  value  rather  than
              determining  it on the fly.  If $MANPATH is prefixed by a colon,
              then the value of the variable is appended to  the  list  deter‐
              mined from the content of the configuration files.  If the colon
              comes at the end of the value in the variable, then  the  deter‐
              mined  list  is appended to the content of the variable.  If the
              value of the variable contains a double  colon  (::),  then  the
              determined  list is inserted in the middle of the value, between
              the two colons.

Now lets go back to Fish. Right now, the first thing that comes to mind if you want to add a directory to the MANPATH is to append it to the end of the array

set -gx MANPATH $MANPATH /my/custom/directory

But this will not have the desired result if MANPATH was not previously set! After running this command man will stop displaying the system manpages that it was displaying before. It will only display manpages from the custom directory.

In this older issue there are some proposed workarounds but none of them are ideal:

 # Put a colon in the directory name
 set -gx MANPATH $MANPATH ":/my/custom/dir"

 # Add an empty element to the array
 set -gx MANPATH $MANPATH "" "/my/custom/dir"

If MANPATH is unset, using these workarounds ensure that MANPATH starts with a colon, which is what we want. But now we will be doing the wrong thing if MANPATH did have a value beforehand! In this case, we will end up with a :: in the middle of the MANPATH, which also has a special meaning.


Is there any big reason for why MANPATH should be an array instead of a regular colon-separated string? Right now Fish makes it easy to iterate over MANPATH or remove paths from the MANPATH but its very hard to just append something to the MANPATH, which I suspect is a much more common thing people will want to do.

@ridiculousfish ridiculousfish added this to the next-2.x milestone May 24, 2015
@ridiculousfish
Copy link
Member

Thanks for the detailed explanation. Maybe we should remove MANPATH from the arrayify-whitelist

@hugomg
Copy link
Contributor Author

hugomg commented May 24, 2015

A quick question: I just found out that

 set -gx MANPATH "$MANPATH:/my/custom/dir"

works fine as a workaround but if I get rid of the quotes it becomes a noop:

 set -gx MANPATH $MANPATH:/my/custom/dir

I thought fish was supposed to expand unset variables to the empty string?


edit: this workaround is actually broken when MANPATH has length > 1 because it will expand to a space-separated list instead of a colon-separated one.

@ridiculousfish
Copy link
Member

See this comment for an explanation of how fish handles unset variables.

@hugomg
Copy link
Contributor Author

hugomg commented May 27, 2015

One possible workaround that seems to work is to set MANPATH to an array of length 1 containing an empty string in the cases when MANPATH is previously unset because when MANPATH always has length >= 1 then doing set -gx MANPATH $MANPATH "/blah" always does the right thing.

if [ 0 -eq (count $MANPATH)]; set -gx MANPATH ""; end

@zanchey
Copy link
Member

zanchey commented Oct 9, 2015

Annoyingly, MANPATH is platform-dependent - the double-colon trick doesn't work on BSD or OS X.

@Lucretiel
Copy link
Contributor

I like @hugomg's idea, because it can be applied in-general to the colon-separation whitelist, without making MANPATH a special snowflake.

@faho
Copy link
Member

faho commented Oct 21, 2015

For $PATH it's completely unnecessary - we always set that, for CDPATH my instinct is that it's more unusual to add to it, so it would still leave MANPATH as a "relatively special snowflake", especially when considering all other variables.

Plus, if I understand @zanchey's comment correctly, an empty element in our listified MANPATH doesn't actually do the same thing on BSD or OSX as it does on Linux, so this still wouldn't work.

The three solutions I can see here

  • Do nothing (i.e. reject the problem)
  • Always set MANPATH to the "default" value as given by manpath (assuming that's available everywhere) - i.e. set -gx MANPATH (manpath | string split ":")
  • Remove MANPATH from the listify list

Of those, the second seems like it has the smallest impact.

@hugomg
Copy link
Contributor Author

hugomg commented Oct 21, 2015

While option 2 is least impactful, is MANPATH really special to justify its inclusion in the listify list?

@krader1961
Copy link
Contributor

This is a (slightly reworded) version of what I said in issue #436 where we're talking about how to handle arrays in exported vars in a more general fasion:

I'm sorry but I don't see any merit to this complaint. The problem is trivially worked around by explicitly testing whether MANPATH is already set. Which, it seems to me, is something you have to do in any event given the semantics of leading versus trailing versus back to back colons. Having said that I'm inclined to support the proposal to remove MANPATH from the list of special-cased env vars.

@Ramblurr
Copy link

So I've landed here because I'm having trouble adding a path to my MANPATH env var. Could someone post a canonical workaround/technique for appending a path to MANPATH?

@Lucretiel
Copy link
Contributor

Currently, MANPATH is an array variable, which means that in fish, it can be set like this:

set -x MANPATH /dir/1 /dir/2

And appended to like this:

set MANPATH $MANPATH /dir/3

This behavior has the issue that there will not be a leading or trailing :, which apparently affects how man looks up manpages. This thread is a proposal to make this no longer the default behavior.

@krader1961 krader1961 modified the milestones: next-major, next-2.x Feb 26, 2016
@dpc
Copy link

dpc commented Mar 25, 2016

I just do:

if test "$MANPATH" = ""
      set -gx MANPATH (manpath)
end

and then:

set -gx MAN_PATH "$HOME/opt/share/man:$MAN_PATH"

Is this reasonable approach?

@faho
Copy link
Member

faho commented Mar 25, 2016

@dpc: No, that is not reasonable. Fish expects every element of $MANPATH to be one directory, what you have done here is set it to one element.

If you have a git fish, you'd want set -gx MANPATH (manpath | string split :) and then set -gx MANPATH $HOME/opt/share/man $MANPATH.

(Your approach might work because I don't think we use MANPATH anywhere ourselves, but it is conceptually wrong)

@dpc
Copy link

dpc commented Mar 25, 2016

OK. so I was totally surprised that MANPATH is also an array.

After more reading, I am totally confused about fish PATH-like stuff handling. Why would MANPATH be handled by an array, while linker paths etc. are not. IMO, having an array types in the language is beneficent, but trying to force it on PATH/MANPATH/CDPATH (anything else?) is just oddity.

Is there any documentation that explains clearly current state of affairs?

@faho
Copy link
Member

faho commented Mar 25, 2016

Is there any documentation that explains clearly current state of affairs?

http://fishshell.com/docs/current/index.html#variables-arrays:

fish automatically creates arrays from the variables PATH, CDPATH and MANPATH when it is started. (Previous versions created arrays from all colon-delimited environment variables.)

This is a whitelist that we hope to never change, to avoid breaking user's scripts.

@dpc
Copy link

dpc commented Mar 25, 2016

@faho: One last question.

Previous versions created arrays from all colon-delimited environment variables.

Which release was that changed in?

@faho
Copy link
Member

faho commented Mar 25, 2016

2.2.0, though I believe it wasn't mentioned in the release notes. This seems like an oversight.

@dpc
Copy link

dpc commented Mar 25, 2016

@faho In the example:

set -gx MANPATH $HOME/opt/share/man $MANPATH

Don't I need to quote "$HOME/opt/share/man" in case $HOME contains a space?

@faho
Copy link
Member

faho commented Mar 25, 2016

@dpc: No. Fish does not split variables after they have been set, so anything that is one element in a variable will be passed as one argument. (Not doing this is possibly the single worst design mistake in classic POSIX shell)

You can set var "SOME VALUE WITH SPACES" and then call e.g. for i in $var; echo $i; end, and it will always call echo once, because it has been passed as one argument once. In a sense fish "keeps" the quotes.

Anyway, this is way out of the scope of this issue. If you have any more questions, feel free to open another one!

@krader1961
Copy link
Contributor

FYI, I have a proof of concept implementation for "tied" variables. This will make it possible to treat MANPATH as a scalar of colon-separated paths while at the same time being tied to fish_manpath which will be an array of the paths. Updating either var will automatically update the other. However, we can only make this change in a major release. So you might want to follow the discussion in issue #4154 about making a fish 3.0 release.

@krader1961
Copy link
Contributor

I'm going to close this as a dup of #4154 since we're definitely going to have tied vars in fish 3.0 and that will solve this issue.

@krader1961 krader1961 removed this from the next-major milestone Jul 21, 2017
@abc-mikey
Copy link

You should really have gone with hugomg's suggestion to set the default MANPAGE to an array containing one item of an empty string, so that adding to MANPAGE would always behave as expected.

Looking forward to fix in 3.0.

@ridiculousfish
Copy link
Member

I took a look at the MANPATH issue described in #2090. The scenario is to append to manpath such that it continues using system paths.

In bash one would write this as export MANPATH="$MANPATH:/new/path". If MANPATH is set, this will append to it. If unset, this will prepend a colon, which is a man-specific indication to use system directories. This syntax doesn't work in fish; the problem is that MANPATH is an array and so "$MANPATH" will have spaces instead of colons.

A "tied variables" approach would allow us to have e.g. fish_manpath as an array that mirrors MANPATH as a colon-separated string. This could be built entirely in fish script. However we would want to do this for all path-like variables, not just MANPATH, and that would be a significant compatibility break which I'm not sure how to manage. Also it has some of the same problems, e.g. the manpath array-variable in zsh has the same problems described here so it's not clear why it even exists.

My proposal here doesn't make the MANPATH situation better or worse; I think the thing to do is punt and just have an easy story for appending to MANPATH, which is this:

set -q MANPATH || set MANPATH ''
set -x MANPATH $MANPATH /new/path

That is not too painful to paste into config.fish.

@bolry
Copy link

bolry commented Mar 3, 2019

With

set -q MANPATH || set MANPATH ''

did you mean

set -q MANPATH; or set MANPATH ''

@faho
Copy link
Member

faho commented Mar 3, 2019

@bolry: Fish 3.0 supports both.

@evanhackett
Copy link

evanhackett commented Nov 21, 2019

I'm still having an issue with an installer setting manpath in a way that breaks my man command so it no longer will search in the default directories. In my case it is opam, so my man command only finds ocaml related searches.

I tried the fixing it with the code snippet above:

set -q MANPATH || set MANPATH ''
set -x MANPATH $MANPATH /new/path

but that doesn't seem to work for me. Any updates on this?

UPDATE: I fixed if by using set -gx instead of set -x

@abc-mikey
Copy link

@evanhackett fyi here is some discussion on the ocaml GitHub about how to handle fish shell:

ocaml/opam#3005

nuxeh added a commit to nuxeh/git-subrepo that referenced this issue Mar 18, 2020
This adds the manual page path for the git-subrepo man page when using
the fish startup script for git subrepo.

This is a not uncontentious issue it seems, in fish shell, see:
fish-shell/fish-shell#2090

This however seems to be the suggested solution, and appears to work
well, with the man page working along with system manpages, and
`manpage` command producing a reasonable result.
nuxeh added a commit to nuxeh/git-subrepo that referenced this issue Mar 19, 2020
This adds the manual page path for the git-subrepo man page when using
the fish startup script for git subrepo.

This is a not uncontentious issue it seems, in fish shell, see:
fish-shell/fish-shell#2090

This however seems to be the suggested solution, and appears to work
well, with the man page working along with system manpages, and
`manpage` command producing a reasonable result.
admorgan pushed a commit to ingydotnet/git-subrepo that referenced this issue Mar 25, 2020
This adds the manual page path for the git-subrepo man page when using
the fish startup script for git subrepo.

This is a not uncontentious issue it seems, in fish shell, see:
fish-shell/fish-shell#2090

This however seems to be the suggested solution, and appears to work
well, with the man page working along with system manpages, and
`manpage` command producing a reasonable result.
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 16, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests