Support compressed man-pages

[vntw]

Fixes #2228

This PR lets man handle the search for man-pages, so it doesn't matter if they're uncompressed, .gz, .bz2, etc. Instead of passing the full path to man, it will only pass the man-page name (e.g. hub-sync). Searching for local man-page files remains unchanged if man is not installed.

It took me some time to understand how everything works together with my changes, e.g. cmd.Exec replacing the current process (at least on mac), man not exiting with code 16 on every platform if the page couldn't be found, and the whole fakebin stuff, so I might've missed something that would've made things easier for me :P

Open questions

1. Before continuing, is this the right direction?
2. fakebin/man and the test scenarios:

When initially running the test suite (after my changes), the scenario for getting help for a command didn't work anymore since man is replaced by the minimal fakebin/man script which didn't do anything special. Even if the original man was the one being used, it wouldn't find any installed man-pages, and since cmd.Exec replaces the current process, the cmd.HelpText() code won't ever be reached (as far as my tests showed). Previously this code was invoked in the error case when the local man-page file couldn't be found (which happened before running the man cmd).

So for now I added generating man-pages to the test script (the only ready-to-use "man-page" that is shipped with the repo is the hub.1.md file) and did some changes to imitate the original man binary to get the tests working again for all commands/subcommands.

I'm not sure this is the best solution (always building man-pages in that case), so I'm open for other ideas if this PR is heading in the right direction.

I hope what I wrote here makes sense or is at least somewhat understandable :P

Thanks!

PS: I also thought about running sth. like

if _, err := man.CombinedOutput(); err != nil {
    return err
}

before running the actual command just to see if it exits with an error so I could return that error and fall back to cmd.HelpText(), but didn't really like running the command basically twice. Maybe you have a different opinion :)

[mislav]

Thank you, but please help me understand first:

• what creates compressed man pages?
• how are their filenames called?
• how does hub's current help system (e.g. hub help hub) not support compressed pages?

I think the effort to fix this is worthwhile (especially if we can simplify the overall approach) but let's first try to make the diff smaller, otherwise I feel that this change is risky.

[mislav]

I don't think we strictly need man pages to be built during tests. We can work around it.

[mislav]

I think that the additions to this fake man executable are superfluous. The only point of this executable is to log its invocation to .history so that the tests can verify that man was called and with what arguments. I don't think that it should try have any output or perform any logic.

[vntw]

Hey thanks for taking the time.

what creates compressed man pages?

Not sure what distro compresses their man-pages, I took the information from the bug issue #2228 and verified that man actually does support it though via man man.

how are their filenames called?

Just like usual, only with the added compression suffix, like hub-sync.1.gz or hub-sync.1.bz2. Right now I don't know all the compressions it supports, just the two I mentioned.

how does hub's current help system (e.g. hub help hub) not support compressed pages?

Currently, hub only searches for hub-%s.1 files in two paths. Since there may be quite some more paths where man files could exist, I decided to let man do its job, i.e. search for the man-pages and deal with the compression (plus all the hidden features that it might have that I've never heard about).

---

But thinking about it again, I have another possible solution: To reduce the amount of changes, we could remove the searching responsibility from man and do it ourselves (again) but this time extend it by looking for more files with a list of known compression suffixes in localManPage. Then the full path could be passed to man (as before) which then deals with the compression. This would remove most of my code in displayManPage, the fakebin/man changes and allow the cmd.HelpText() fallback, which also helps the tests.

Thoughts/disadvantages that come to mind:

• This assumes that the compressed man-pages also live in one of those two paths. Maybe @eli-schwartz can shed some light on that, i.e. are the compressed man-page files in the same directory or are they somewhere else? That might render this alternative approach "useless"
• We have to keep a list of supported compressions by man (and have to find out that list :D) - but I guess it won't change very much
• Theoretically it could happen that man is not installed but we find a compressed man-page. In that case whatever is in $PAGER or less may not be able to handle it since it's a binary file. Do we care in that case?

Anyway I hope this helped you, let me know if I can help further

[eli-schwartz]

• what creates compressed man pages?

Most linux distributions will have a post-process step on any packaged man pages, which runs, for example, gzip /path/to/manpage before creating the distribution package.

• how are their filenames called?

Always the same directory and filename as the current version of the manpage, except with a file extension denoting the compression format.

• how does hub's current help system (e.g. hub help hub) not support compressed pages?

As I initially specified in #2228, hub tries to find the filename and call man using the filename, which doesn't work because the file has been renamed with a file extension and thus hub cannot find it (but man can, iff you use the manual page name hub, not the filename /path/to/hub.1).

[eli-schwartz]

@venyii, thanks for working on this!

Right now I don't know all the compressions it supports, just the two I mentioned.

From the man man manpage:

These utilities support compressed source nroff files having, by default, the extensions of .Z, .z and .gz. It is possible to deal with any compression extension, but this information must be known at compile time.

It's not picky, users are permitted to paint the bikeshed any color they like. :)

Currently, hub only searches for hub-%s.1 files in two paths. Since there may be quite some more paths where man files could exist, I decided to let man do its job, i.e. search for the man-pages and deal with the compression (plus all the hidden features that it might have that I've never heard about).

I would favor this option, since it seems to be what the git program does too, but really the truth is that hub only currently supports two directories, and could continue to support two directories without caring what additional directories man is configured to search -- on the assumption that any hub installation will still use the default package-manager controlled location.

• This assumes that the compressed man-pages also live in one of those two paths. Maybe @eli-schwartz can shed some light on that, i.e. are the compressed man-page files in the same directory or are they somewhere else? That might render this alternative approach "useless"

Again, compressed man pages are no different from uncompressed ones, they go in the same default search directory path except the file extensions denote their compressed status.

• We have to keep a list of supported compressions by man (and have to find out that list :D) - but I guess it won't change very much

You could look at the currently supported list in https://git.savannah.gnu.org/cgit/man-db.git/tree/include/comp_src.h.in

It depends on how your distro has configured man-db.

• Theoretically it could happen that man is not installed but we find a compressed man-page. In that case whatever is in $PAGER or less may not be able to handle it since it's a binary file. Do we care in that case?

Even if it is uncompressed, you wouldn't want to read it with less since nroff formatting codes are pretty obtuse. On the other hand, if the user has lesspipe.sh set up, then it will both decompress the file on the fly and pass it through the groff command to format it. This will still be inferior to using a proper manpage reader like man, but on the other hand, which system has groff installed and not man? Windows probably has neither, and most Linuxes definitely have both...

[mislav]

Thank you for taking the time to explain things to me.

I'm trying to remember why I made the current help logic way more complicated than it should have been. Basically, running hub help hub, hub help pr, or hub help hub-checkout should be equivalent to running man hub, man hub-pr, man hub-checkout, respectively. (Additionally, to match git's behavior, hub pr --help should open the hub-pr(1) man page, but right now it only prints the usage synopsis for hub pr.)

However, there are scenarios where simply delegating to man would fail:

• In development, having bin/hub help pr open the system's hub-pr(1) man page would not be a great experience since the document would either be outdated (it might belong to a system-wide installation of hub) or missing (there's no system-wide hub).

  Potential solution: perhaps hub can detect that it's running in "development" mode and set a custom MANPATH prior to invoking man.

• Hub might have been installed in a nonstandard path, thus its man pages ending up in a location other than the typical MANPATH (/usr/share/man:/usr/local/share/man).

• Some OSs don't have man out of the box; most notably Windows. On those platforms, we show the plain text help instead.

In any case, I'm definitely up for simplifying the approach: let's delegate as much as we can to man internal mechanisms, let's not try to resolve the full path to the man page, but let's let's stay conscious of scenarios when man hub-<command> might be unavailable.

[mislav]

@venyii Thank you for the work in this PR. I've decided to go a different route, most notably a simpler approach (just call man hub-<cmd>). Your code helped me get on the right track.

@eli-schwartz Thank you for your thoughts 🙇

[eli-schwartz]

@mislav Thanks, that looks like it should work nicely. Looking forward to trying out the new release.

[vntw]

Hey thanks for the PR @mislav and sorry for the inactivity, I was sick some time and didn't have a clear solution yet after the more detailed discussion. But I'm glad that this PR at least helped a bit!