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

Generate manpage(s) for pineappl binary #145

Open
1 of 2 tasks
cschwan opened this issue Jun 11, 2022 · 26 comments
Open
1 of 2 tasks

Generate manpage(s) for pineappl binary #145

cschwan opened this issue Jun 11, 2022 · 26 comments
Assignees
Labels
documentation Improvements or additions to documentation

Comments

@cschwan
Copy link
Contributor

cschwan commented Jun 11, 2022

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

  • installed man pages, so that man pineappl and pineappl help both work
  • more detailed man pages showing examples and all relevant details
@cschwan cschwan added the documentation Improvements or additions to documentation label Jun 11, 2022
@cschwan cschwan self-assigned this Jun 11, 2022
@cschwan
Copy link
Contributor Author

cschwan commented Jul 28, 2022

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 clap-rs/clap#3365.

@cschwan
Copy link
Contributor Author

cschwan commented Feb 1, 2023

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

  • Commit 76a2200 adds a preliminary help subcommand that behaves very similarly to git: pineappl --help or pineappl convolute --help still prints help to the terminal for either the main command or a specific subcommand.
  • New is the following behaviour: pineappl help opens the manpage for the main command and pineappl help convolute opens the manpage for a specific command. This works also with nested subcommands (just as with --help), pineappl help analyze ckf opens the ckf subcommand of analyze of pineappl.

@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
Copy link
Member

First things first: it works perfectly :)

A couple of suggestions:

  • now that they are displayed as proper man pages, a long description might be useful, and possibly also a brief example section (I use it very often)
  • 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

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
Copy link
Member

alecandido commented Feb 1, 2023

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
Copy link
Member

scarlehoff commented Feb 1, 2023

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
Copy link
Member

alecandido commented Feb 1, 2023

@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
Copy link
Member

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
Copy link
Contributor Author

cschwan commented Feb 1, 2023

@scarlehoff try:

cargo install --git https://github.com/NNPDF/pineappl.git

@cschwan
Copy link
Contributor Author

cschwan commented Feb 1, 2023

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
Copy link
Member

alecandido commented Feb 1, 2023

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
Copy link
Member

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
Copy link
Contributor Author

cschwan commented Feb 1, 2023

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

@cschwan
Copy link
Contributor Author

cschwan commented Feb 1, 2023

Commit 8ca0cb1 should hopefully fix the problem on MacOS.

@scarlehoff
Copy link
Member

Commit 8ca0cb1 should hopefully fix the problem on MacOS.

It has fixed that problem

Screenshot 2023-02-01 at 15 49 08

@scarlehoff
Copy link
Member

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
Copy link
Contributor Author

cschwan commented Feb 2, 2023

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

@scarlehoff
Copy link
Member

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

The fact that it looks at dev sounds weird

@alecandido
Copy link
Member

The fact that it looks at dev sounds weird

Not really...

.args(["/dev/stdin"])

@cschwan
Copy link
Contributor Author

cschwan commented Feb 2, 2023

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
Copy link
Contributor Author

cschwan commented Feb 2, 2023

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
Copy link
Member

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
Copy link
Member

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
Copy link
Contributor Author

cschwan commented Feb 2, 2023

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
Copy link
Contributor Author

cschwan commented Jul 18, 2023

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

@cschwan
Copy link
Contributor Author

cschwan commented Aug 31, 2023

Since commit 087b7b7 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
Copy link
Contributor Author

cschwan commented Jan 31, 2024

Commit bdea693 removes the wrong hyphens in the SYNOPSIS.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

No branches or pull requests

3 participants