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

enforce limit of one subcommand, list named_args #1348

Merged
merged 1 commit into from Apr 24, 2024

Conversation

rrotter
Copy link
Contributor

@rrotter rrotter commented Apr 24, 2024

  • Add named_args so Homebrew::Completions can discover subcommands
  • raise an error if args.named.size > 1, unless we're running brew bundle exec: this fails loudly on some commands that are currently accepted and may appear valid but don't do what you'd expect. Each of the below commands currently execute the first subcommand and ignore the second silently:
    • brew bundle install my_package
    • brew bundle install cleanup (not to be confused with brew bundle install --cleanup!)
    • brew bundle cleanup install
  • note: cannot use the max: 1 annotation for named args because this would break brew bundle exec

I wrote this commit as part of my abortive effort on zsh completion. Sending the PR anyway, because imo it's still an improvement to usability and will be of some assistance if anyone continues working on completions for brew bundle.

- adding named_args will let Homebrew::Completions discover subcommands
- limiting subcommmands stops user from adding arguments we won't execute anyway
  - can't use `max: 1` because this would break `brew bundle exec`
Copy link
Member

@MikeMcQuaid MikeMcQuaid left a comment

Choose a reason for hiding this comment

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

Great idea, thanks again @rrotter!

CC @dduugg who may find this interesting.

@MikeMcQuaid MikeMcQuaid merged commit ca1ff53 into Homebrew:master Apr 24, 2024
4 checks passed
@rrotter rrotter deleted the cmd-completion branch April 24, 2024 16:16
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.

None yet

2 participants