search

NNPDF / pineappl

PineAPPL is not an extension of APPLgrid
https://nnpdf.github.io/pineappl/
GNU General Public License v3.0
12 stars 3 forks source link

Generate manpage(s) for `pineappl` binary #145

Open cschwan opened 2 years ago

cschwan commented 2 years ago

Use https://docs.rs/clap_mangen/. Ideally, we'd like to have

cschwan commented 2 years ago

Small update on this: clap_mangen makes this quite easy, however, it seems currently it doesn't document subcommands, at least not automatically. Since all of our functionality is contained in subcommand we can't use it yet. See also https://github.com/clap-rs/clap/issues/3365.

cschwan commented 1 year ago

The situation here is much better than expected, I just didn't look far enough: https://github.com/clap-rs/clap/discussions/3603 contains the missing information.

@scarlehoff @AleCandido @felixhekhorn @andreab1997 could try this out and give me some feedback? Does it work on Mac?

My idea would be to print a short help with --help and help should print all details, examples and hopefully some of the documentation that's online.

alecandido commented 1 year ago

First things first: it works perfectly :)

A couple of suggestions:

And one observation: most likely this depends on clap, but before the output of PineAPPL was nicely colored, while now it is underlined and bold (so term codes are used), but it is not colored any longer.

alecandido commented 1 year ago

Another option you may consider is to make a PR to tldr, it can be useful to have a very short list of examples there, this I use even more than man pages examples, since the latter are not always present.

A couple of relevant examples:

scarlehoff commented 1 year ago

Does it work on Mac?

--help does work, man pineappl does not

pineappl help produces: error: Found argument 'help' which wasn't expected, or isn't valid in this context

This is with 0.5.9 from cargo install pineappl_cli.

alecandido commented 1 year ago

@scarlehoff did you install from master? i.e. cargo install --path pineappl_cli from Git root

man pineappl does not work because man pages are generated, not installed.

EDIT: I'm not even sure Cargo can install files other than executables during cargo install

scarlehoff commented 1 year ago

EDIT: I'm not even sure Cargo can install files other than executables during cargo install

I see.

No, from whatever crate cargo has. Because it was from almost a year ago I thought it was already in the official version.

cschwan commented 1 year ago

@scarlehoff try:

cargo install --git https://github.com/NNPDF/pineappl.git
cschwan commented 1 year ago

And one observation: most likely this depends on clap, but before the output of PineAPPL was nicely colored, while now it is underlined and bold (so term codes are used), but it is not colored any longer.

That's the change from clap-v3 to v4, see here:

alecandido commented 1 year ago

Ok, it looks like we should track

One of the mentioned options is also to have presets, and since PineAPPL does not use any other colors besides clap, a preset would be perfect for it.

scarlehoff commented 1 year ago

I'm afraid it doesn't work in mac

pineappl help
/usr/bin/man: illegal option -- l
Usage:
 man [-adho] [-t | -w] [-M manpath] [-P pager] [-S mansect]
     [-m arch[:machine]] [-p [eprtv]] [mansect] page [...]
 man -f page [...] -- Emulates whatis(1)
 man -k page [...] -- Emulates apropos(1)

I cannot find an equivalent for linux's -l

cschwan commented 1 year ago

@scarlehoff thanks, I was afraid that would happen. I'll try to fix it!

cschwan commented 1 year ago

Commit 8ca0cb1ff0b2ff38f5133f5f69aed7f5313cb273 should hopefully fix the problem on MacOS.

scarlehoff commented 1 year ago

Commit 8ca0cb1 should hopefully fix the problem on MacOS.

It has fixed that problem

Screenshot 2023-02-01 at 15 49 08

scarlehoff commented 1 year ago

Let me add, in case it is useful, some of the error msgs I'm getting (I didn't realize before because... I didn't close the empty man page until now.

zcat: can't stat: /dev/fd/sh-thd-1675288810 (/dev/fd/sh-thd-1675288810.gz): No such file or directory
cschwan commented 1 year ago

That looks like it expects a gzipped file instead of an uncompressed one and it should be solved by installing the files.

scarlehoff commented 1 year ago

Which files? I'm doing cargo install git...

The fact that it looks at dev sounds weird

alecandido commented 1 year ago

The fact that it looks at dev sounds weird

Not really... https://github.com/NNPDF/pineappl/blob/8ca0cb1ff0b2ff38f5133f5f69aed7f5313cb273/pineappl_cli/src/help.rs#L49

cschwan commented 1 year ago

The fact that it looks at dev sounds weird

pineappl help ... works by internally writing the man page to stdout and configuring stdout to pipe into man's stdin, so in terms of shell commands basically pineappl ... | man -l -. The switch -l unfortunately isn't supported by MacOS, as you saw, so I tried pineappl ... | man /dev/stdin which should do effectively the same. The file /dev/stdin, however, means something different for each program (each programs' stdin FD), which MacOS accomplishes by translating it in your case to /dev/fd/sh-thd-1675288810. It seems that MacOS interprets this as: 'open the man page for this file' and it requires the man page for that file to be gzipped (with a .gz ending) which of course doesn't exist. I might have to give it a relative path starting with ./ to prevent that, but that means I can't use pipes anymore. Since we'd like to install the manpages anyway so that man pineappl-convolute works similar to git it'll be solved when I'll do that.

cschwan commented 1 year ago

what about installing (or provide a way/subcommand to install) the generated manpages in $HOME/.local/share/man? Ubuntu at least is adding it to $MANPATH, and in this way you could retrieve them with man pineappl convolute

I think we can avoid setting any environment variables since usually man also goes through $PATH, and for each directory $DIR in $PATH it looks for man pages in $DIR/../share/man.

alecandido commented 1 year ago

I was mainly suggesting to use $HOME/.local/share/man for installation, because it is something that is probed by default, rather than setting $MANPATH in PineAPPL

scarlehoff commented 1 year ago

Thanks for the explanation! I see now what you mean by installing.

fwiw, doing anything | man seems to work in macos but I prefer the installation solution mainly because when I want to know how a program works my main reflex is to do man program.

cschwan commented 1 year ago

I was mainly suggesting to use $HOME/.local/share/man for installation, because it is something that is probed by default, rather than setting $MANPATH in PineAPPL

I see! However, in pineappl's case $HOME/.cargo/share/man would be more appropriate (actually $(dirname $(command -v pineappl))/../share/man), I think, as it's possible that $PATH doesn't include ~/.local (unlikely, I know, but possible).

cschwan commented 1 year ago

We'll need #236 to implement the remaining changes.

cschwan commented 11 months ago

Since commit 087b7b70682a658ba64b019917d8632019e9587f it's possible to install man pages, simply run

mkdir -p $(dirname $(which pineappl))/../share/man/man1/
cargo xtask install-manpages $(dirname $(which pineappl))/../share/man/man1/

and at least on Linux man pineappl or man pineappl-convolute works as expect from git, for instance. With commit 4d198f1 pineappl help now relies on these installed man pages

What remains is to improve the usability and error handling.

cschwan commented 6 months ago

Commit bdea6932478bebf31ce7a4ffa9623c190b5dd133 removes the wrong hyphens in the SYNOPSIS.