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

Show command help in $PAGER or less #3434

Closed
mrinalwadhwa opened this issue Sep 7, 2022 · 30 comments · Fixed by #3622
Closed

Show command help in $PAGER or less #3434

mrinalwadhwa opened this issue Sep 7, 2022 · 30 comments · Fixed by #3622

Comments

@mrinalwadhwa
Copy link
Member

mrinalwadhwa commented Sep 7, 2022

Currently:

We have many help commands that show a long help, for example ockam secure-channel --help. This hides the command syntax above the fold.

Desired:

  1. Detect if $PAGER is set, if so invoke that and send all help output to it.
  2. If $PAGER is not set, default to less

We love helping new contributors!
If you have questions or need help as you work on your first Ockam contribution, please leave a comment on this discussion.
If you're looking for other issues to contribute to, checkout this discussion and labels - good first issue or help wanted

@mrinalwadhwa mrinalwadhwa changed the title Show command help in $PAGER Show command help in $PAGER or less Sep 7, 2022
@phillyphil91
Copy link
Contributor

phillyphil91 commented Sep 9, 2022

Hey, i would like to take a look at this, as a first contribution. I am not entirely sure, I understand the details fully though.
At what stage should the check for $PAGER happen? And once that's set to less instead of the normal display of the help commands, this should be "routed" as input to less ?

Edit: Found HELP_DETAIL or rather the place where help::template etc. is defined

@phillyphil91
Copy link
Contributor

In general it looks like only man uses the $MANPAGER or $PAGER env. So I guess I would have to find a way for the help output to use the tools specified by those ENVs as well. I can take a look if clap allows this or if there is any other way to do this.
If I'm going off into the wrong direction, please let me know.

@mrinalwadhwa
Copy link
Member Author

@phillyphil91 thank you for spending time on this.
I think that is right direction.
We need to find a way in clap to override where the output goes.

@mrinalwadhwa
Copy link
Member Author

The clap community has a friendly discussions page here https://github.com/clap-rs/clap/discussions so asking there for some guidance might be helpful as well.

@phillyphil91
Copy link
Contributor

The clap community has a friendly discussions page here https://github.com/clap-rs/clap/discussions so asking there for some guidance might be helpful as well.

Good idea. I will have a look there.

@phillyphil91 thank you for spending time on this. I think that is right direction. We need to find a way in clap to override where the output goes.

Clap allows you to write to stdout or to a io::Write object.
image
The problem then still is how to get this into less for example. Maybe it would be easier to just allow the user to do ockam secure-channel --help | less if they want to. But i will have another look.

@mrinalwadhwa
Copy link
Member Author

I was thinking if we can capture the help output, then maybe we can execute echo "$output" | less in a subprocess

@phillyphil91
Copy link
Contributor

i was thinking the same, but was not able to get anything meaningful working yet. I'll have another look at spawning (sub)processes. Thanks

@phillyphil91
Copy link
Contributor

phillyphil91 commented Sep 10, 2022

I found something that allows you to open a sub-process for example for less (How to run ‘less’ command for command line application) and wait until the sub-process exits but the question is if it's possible run this as a custom function or something if the help command is used. Disabling the help sub command is possible in clap but then help would have to be implemented ourselves alongside clap, which would mean help::template etc. could not be used.

But I opened a question in the clap forum to see if they have any idea: clap-rs/clap#4200

@phillyphil91
Copy link
Contributor

So according to clap-rs/clap#4201, using a pager for the help output is currently being investigated by the clap team. So maybe this issue can be closed until further advancement is done on their side? Or are there any other suggestions?

@mrinalwadhwa
Copy link
Member Author

@phillyphil91 Thank you for exploring this and engaging with the Clap community on that thread. I'll leave the issue open so we remember to come back to it.

In the meantime, maybe we should invest effort in making -h and --help should short and long help respectively. I feel this distinction is hard to discover for new users.

@epage
Copy link

epage commented Sep 12, 2022

In the meantime, maybe we should invest effort in making -h and --help should short and long help respectively. I feel this distinction is hard to discover for new users.

In clap v4 (working on wrapping up the final documentation for it), we've updated the help output in hopes to reduce confusion on this. See clap-rs/clap#4159

@mrinalwadhwa
Copy link
Member Author

@epage Thank you for that context. clap v4 sounds exciting, looking forward to upgrading to it!

@phillyphil91
Copy link
Contributor

@phillyphil91 Thank you for exploring this and engaging with the Clap community on that thread. I'll leave the issue open so we remember to come back to it.

In the meantime, maybe we should invest effort in making -h and --help should short and long help respectively. I feel this distinction is hard to discover for new users.

So create different help messages according to if -h or --help was used?

And the -h one would only show the most important help info for the command and maybe refer to --help for a more detailed one. Or what were you thinking? @mrinalwadhwa

@mrinalwadhwa
Copy link
Member Author

@phillyphil91 that's exactly what I was thinking. We maybe able to control it with a help_template or long_help related options in clap.

@epage
Copy link

epage commented Sep 15, 2022

If you want, with arg.help and arg.long_help, you can get the clap v4 behavior today.

If you aren't defining your own help, it can most likely work by doing something like cmd.mut_arg("help", |arg| arg.help("...").long_help("..."))

@cailloumajor
Copy link

Hello, maybe it can help, the great https://github.com/sharkdp/bat tool automatically (I guess it is configurable) uses pager if output is too long.

@mrinalwadhwa
Copy link
Member Author

@cailloumajor 👋 If you have thoughts / ideas on how this could work until clap has it baked in. We'd love to learn more / explore them with you.

@epage
Copy link

epage commented Sep 16, 2022

Some pre-built pager options

  • bat though I'm unsure if there how heavy weight it is just for a help pager
  • minus is focused on being a pager library and has feature flags to trim things down
  • pager works by forking your process so any output is automatically sent to an external pager. You need to make sure you do any terminal capability detection before the fork occurs
    • If colored help is enabled, do the detection yourself and pass it into Command::color
    • If wrap_help is enabled, call terminal_size directly and pass that into Command::term_width
    • Hmm, doesn't look like this handles quoted strings correctly

If you go the route of writing your own pager, I've compared different pager implementations for my experiment with a git tool and to prepare for integrating a pager into clap.

@phillyphil91
Copy link
Contributor

phillyphil91 commented Sep 16, 2022

If you want, with arg.help and arg.long_help, you can get the clap v4 behavior today.

If you aren't defining your own help, it can most likely work by doing something like cmd.mut_arg("help", |arg| arg.help("...").long_help("..."))

but help and long_help only exists for arguments right? not for commands or subcommands. I can't set those for running subcommands like ockam secure-channel --help for example?

or would that be achievable via
cmd.mut_arg("help", |arg| arg.help("...").long_help("..."))?

@epage
Copy link

epage commented Sep 16, 2022

If you want the clap v4 behavior where the description for -h, --help adapts a hint per shot or long help, yes, you could call mut_arg on each Command to set that up.

ripgrep and bat take a different approach where they put it in either the command's about and long_about or in after_help and after_long_help.

@phillyphil91
Copy link
Contributor

Sorry for being so persistent about this. But here is what I tried doing from my understanding (see mut_arg:
image
But this "only" changes the description of the -h and --help flag:
image
And:
image
But what I was hoping to do is have the actual behavior of the entire help messages that's printed adjust to -h vs --help
So e.g. -h would show:
image
And --help would show the same plus the whole about + examples etc:
image

I'm not really sure if that is achievable. Allow the user for a short help message with -h and a longer one with examples etc. with --help .
As you can see help_template is being used and I also wonder if that can be set for whether the short or long help argument was used.

@epage
Copy link

epage commented Sep 17, 2022

I think what you are wanting to set is after_long_help. That will allow you to add text after the auto-generated help content but only for --help / help subcommand.

@adrianbenavides adrianbenavides added the hacktoberfest Apply to issues you want contributors to help with label Oct 3, 2022
@hargut
Copy link
Contributor

hargut commented Oct 8, 2022

The above PR implements the approach of using after_long_help which I do think is a good start to address this topic. Nevertheless it is another solution as the requested $PAGER usage.

Personally I'm not a fan of using pagers as default in help, as it typically requires to quit that pager. Most of the time when help is displayed options are either missing, typos or order issues occurred, therefore the short help should be fine for the larger portion of the cases.

@hargut
Copy link
Contributor

hargut commented Oct 8, 2022

To address the issue of the early line wraps I've opened a ticket & PR on clap.
clap-rs/clap#4360 # issue
clap-rs/clap#4361 # pr

@hargut
Copy link
Contributor

hargut commented Oct 9, 2022

The clap PR was accepted and the version has been updated. This was included in #3622

@adrianbenavides adrianbenavides linked a pull request Oct 11, 2022 that will close this issue
4 tasks
@mergify mergify bot closed this as completed in #3622 Oct 12, 2022
@mrinalwadhwa mrinalwadhwa reopened this Oct 12, 2022
@mariannegoldin
Copy link

Hi there! I am working with some new contributors to open source to get them setup on this repo and looking for a good first issue.

Given the update in #3622 - is this issue still open as written?

@mrinalwadhwa
Copy link
Member Author

@mariannegoldin yes, we've made progress in breaking out long_about and after_long_help but we don't yet have the ability to show help inside a pager so this is still open.

long_about = docs::about(LONG_ABOUT),
after_long_help = docs::after_help(AFTER_LONG_HELP)

@mariannegoldin
Copy link

@mariannegoldin yes, we've made progress in breaking out long_about and after_long_help but we don't yet have the ability to show help inside a pager so this is still open.

long_about = docs::about(LONG_ABOUT),
after_long_help = docs::after_help(AFTER_LONG_HELP)

Thanks much! This will be good context to look at in determining if we are able to take this issue on.

@deebrecke
Copy link

This is the issue that my team has chosen for our first attempt. We are students and this is our first open-source project. If there is any additional information you can provide for us, we would greatly appreciate it.

i-b-o-t added a commit to i-b-o-t/ockam that referenced this issue Jun 1, 2023
using $PAGER if set, defaulting to `less` and falling back to `more`
or no pagination if no pager is available.

Addresses build-trust#3434
i-b-o-t added a commit to i-b-o-t/ockam that referenced this issue Jun 1, 2023
using $PAGER if set, defaulting to `less` and falling back to `more`
or no pagination if no pager is available.

Addresses build-trust#3434
i-b-o-t added a commit to i-b-o-t/ockam that referenced this issue Jun 2, 2023
using $PAGER if set, defaulting to `less` and falling back to `more`
or no pagination if no pager is available.

Addresses build-trust#3434
i-b-o-t added a commit to i-b-o-t/ockam that referenced this issue Jun 2, 2023
using $PAGER if set, defaulting to `less` and falling back to `more`
or no pagination if no pager is available.

Addresses build-trust#3434
i-b-o-t added a commit to i-b-o-t/ockam that referenced this issue Jun 2, 2023
using $PAGER if set, defaulting to `less` and falling back to `more`
or no pagination if no pager is available.

Addresses build-trust#3434
i-b-o-t added a commit to i-b-o-t/ockam that referenced this issue Jun 6, 2023
using $PAGER if set, defaulting to `less` and falling back to `more`
or no pagination if no pager is available.

Addresses build-trust#3434
i-b-o-t added a commit to i-b-o-t/ockam that referenced this issue Jun 6, 2023
using $PAGER if set, defaulting to `less` and falling back to `more`
or no pagination if no pager is available.

Addresses build-trust#3434
i-b-o-t added a commit to i-b-o-t/ockam that referenced this issue Jun 6, 2023
using $PAGER if set, defaulting to `less` and falling back to `more`
or no pagination if no pager is available.

Addresses build-trust#3434
i-b-o-t added a commit to i-b-o-t/ockam that referenced this issue Jun 6, 2023
using $PAGER if set, defaulting to `less` and falling back to `more`
or no pagination if no pager is available.

Addresses build-trust#3434
i-b-o-t added a commit to i-b-o-t/ockam that referenced this issue Jun 6, 2023
using $PAGER if set, defaulting to `less` and falling back to `more`
or no pagination if no pager is available.

Addresses build-trust#3434
i-b-o-t added a commit to i-b-o-t/ockam that referenced this issue Jun 6, 2023
using $PAGER if set, defaulting to `less` and falling back to `more`
or no pagination if no pager is available.

Addresses build-trust#3434
i-b-o-t added a commit to i-b-o-t/ockam that referenced this issue Jun 6, 2023
using $PAGER if set, defaulting to `less` and falling back to `more`
or no pagination if no pager is available.

Addresses build-trust#3434
i-b-o-t added a commit to i-b-o-t/ockam that referenced this issue Jun 7, 2023
using $PAGER if set, defaulting to `less` and falling back to `more`
or no pagination if no pager is available.

Addresses build-trust#3434
adrianbenavides pushed a commit that referenced this issue Jun 8, 2023
using $PAGER if set, defaulting to `less` and falling back to `more`
or no pagination if no pager is available.

Addresses #3434
@mrinalwadhwa
Copy link
Member Author

Landed in #5049
Thank you @i-b-o-t and everyone who helped along the way 🥳

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

Successfully merging a pull request may close this issue.

8 participants