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

feat: add support for custom help triggers #106

Merged
merged 1 commit into from
Nov 13, 2023

Conversation

aminya
Copy link
Contributor

@aminya aminya commented Sep 1, 2021

Fixes #30
Related to #53
Related to #119

This adds support for custom help triggers using #[argh(help_triggers("-h", "--help", "help"))].

For example,

/// Height options
#[derive(FromArgs)]
#[argh(help_triggers("-h", "--help", "help"))]
struct Height {
    /// how high to go
    #[argh(option)]
    height: usize,
}

Allows triggering help with -h, --help or help

Usage: test_arg_0 --height <height>

Height options

Options:
  --height          how high to go
  -h, --help, help  display usage information

@google-cla
Copy link

google-cla bot commented Sep 1, 2021

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers

ℹ️ Googlers: Go here for more info.

@aminya
Copy link
Contributor Author

aminya commented Sep 1, 2021

@googlebot I signed it!

jirutka added a commit to jirutka/argh that referenced this pull request Jan 9, 2023
Copy link

@LeoDog896 LeoDog896 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

Copy link
Collaborator

@sadmac7000 sadmac7000 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we can change something this fundamental without having at least the option for the old behavior. My preference would be that it default to off.

@aminya
Copy link
Contributor Author

aminya commented Oct 19, 2023

@sadmac7000 I submitted this pull request in 2021. After these years, is it worth making this simple change configurable? If I was the maintainer, I would have merged it and bumped up the major version.

@sadmac7000
Copy link
Collaborator

argh exists specifically to implement the Fuchsia command line argument guidelines. Breaking behaviors outside of those guidelines need more than just a major revision bump. They need an update to the guidelines or to be disabled by default.

@aminya
Copy link
Contributor Author

aminya commented Nov 2, 2023

@sadmac7000 Where is the Fuschia documentation so that I can make the same pull request for that?

@sadmac7000
Copy link
Collaborator

The guidelines are documented in Google's git hosting here: https://fuchsia.googlesource.com/fuchsia/+/refs/heads/main/docs/development/api/cli.md

@aminya aminya changed the title Add -h as a short alias for --help feat: add support for custom help triggers Nov 12, 2023
@aminya
Copy link
Contributor Author

aminya commented Nov 12, 2023

@sadmac7000 Reimplemented the help trigger logic to make it configurable through attributes.

@aminya aminya force-pushed the help-short-alias branch 2 times, most recently from d60c36c to b8087a0 Compare November 12, 2023 14:16
@sadmac7000
Copy link
Collaborator

Looks good. Looks like there's a merge conflict. Mind rebasing?

@aminya
Copy link
Contributor Author

aminya commented Nov 12, 2023

@sadmac7000 You're welcome. The branch is already rebased. I noticed the GitHub UI can be outdated, so maybe refresh your page.

@sadmac7000
Copy link
Collaborator

It's still claiming the branch cannot be rebased due to conflicts and won't let me merge. I refreshed a few times. I'll look again later in case it's a transient error.

@aminya aminya closed this Nov 13, 2023
@aminya aminya reopened this Nov 13, 2023
@aminya
Copy link
Contributor Author

aminya commented Nov 13, 2023

@sadmac7000 I gave it a shock. Check if it resolved the issue. Haha

@sadmac7000
Copy link
Collaborator

I had to pull it down and manually rearrange it. In the future, please rebase your changes when you need to update rather than merging in upstream. It confuses github.

@sadmac7000 sadmac7000 merged commit f1037ba into google:master Nov 13, 2023
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Offer ability to short circuit help output (Ex: -h)
4 participants