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

expose ValidateRequiredFlags and ValidateFlagGroups #1760

Merged
merged 1 commit into from
Sep 27, 2022
Merged

expose ValidateRequiredFlags and ValidateFlagGroups #1760

merged 1 commit into from
Sep 27, 2022

Conversation

skeetwu
Copy link
Contributor

@skeetwu skeetwu commented Jul 21, 2022

Hi, dear reviewers,

In my project, there is one complex command with a lot of flags, we need to set part of them as required, and other flags are optional, all optional flags were defined from an API server. I have implemented this feature to add dynamic flags in issue #1758.
as talked with @marckhouzam in the last issue, it looks like my method is on the right way.

In the next work, I add some flags in init() function and call MarkFlagRequired to mark them as required flags. but, because I have set DisableFlagParsing as true, and need to call func (c *Command) ParseFlags(args []string) error to parse all flags manually, the function which validates the required flag will not be called.

for now, I have to set up a map and store all required flags, then check if exist in arguments one bye one before ParseFlags(). It works but looks not graceful.

I went through the source code of Cobra, and it looks like I need to call validateRequiredFlags() and validateFlagGroups() manually after ParseFlags(args []string), and function validateRequiredFlags() was called by Execute(), so I created this PR to expose these two functions.

Thx for reviewing this PR.

B&R,
Skeet

@CLAassistant
Copy link

CLAassistant commented Jul 21, 2022

CLA assistant check
All committers have signed the CLA.

@github-actions github-actions bot added the size/S Denotes a PR that chanes 10-24 lines label Jul 21, 2022
@github-actions
Copy link

The Cobra project currently lacks enough contributors to adequately respond to all PRs. This bot triages issues and PRs according to the following rules:

  • After 60d of inactivity, lifecycle/stale is applied. - After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied and the PR is closed.
    You can:
  • Make a comment to remove the stale label and show your support. The 60 days reset. - If a PR has lifecycle/rotten and is closed, comment and ask maintainers if they'd be interseted in reopening.

@skeetwu
Copy link
Contributor Author

skeetwu commented Sep 20, 2022

Hi, exposing these two functions is more convenient and does not conflict with other parts. I have changed it in my local workspace, it works well.
please help to review.

@marckhouzam marckhouzam added the area/flags-args Changes to functionality around command line flags and args label Sep 23, 2022
@marckhouzam marckhouzam added this to the 1.6.0 milestone Sep 23, 2022
@marckhouzam
Copy link
Collaborator

This seems reasonable to give users more flexibility in handling flags.

command.go Outdated Show resolved Hide resolved
Copy link
Collaborator

@marckhouzam marckhouzam left a comment

Choose a reason for hiding this comment

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

Thanks @skeetwu !

@marckhouzam marckhouzam merged commit fce8d8a into spf13:main Sep 27, 2022
@skeetwu
Copy link
Contributor Author

skeetwu commented Sep 28, 2022

@marckhouzam thx for reviewing and helping to merge the code.
I feel so glad to contribute code to a so popular project. 😃

jimschubert added a commit to jimschubert/cobra that referenced this pull request Oct 3, 2022
* main: (39 commits)
  Add '--version' flag to Help output (spf13#1707)
  Expose ValidateRequiredFlags and ValidateFlagGroups (spf13#1760)
  Document option to hide the default completion cmd (spf13#1779)
  ci: add workflow_dispatch (spf13#1387)
  add missing license headers (spf13#1809)
  ci: use action/setup-go's cache (spf13#1783)
  Adjustments to documentation (spf13#1656)
  Rename Powershell completion tests (spf13#1803)
  Support for case-insensitive command names (spf13#1802)
  Deprecate ExactValidArgs() and test combinations of args validators (spf13#1643)
  Use correct stale action `exempt-` yaml keys (spf13#1800)
  With go 1.18, we must use go install for a binary (spf13#1726)
  Clarify SetContext documentation (spf13#1748)
  ci: test on Golang 1.19 (spf13#1782)
  fix: show flags that shadow parent persistent flag in child help (spf13#1776)
  Update gopkg.in/yaml.v2 to gopkg.in/yaml.v3 (spf13#1766)
  fix(bash-v2): activeHelp length check syntax (spf13#1762)
  fix: correct command path in see_also for YAML doc (spf13#1771)
  build(deps): bump github.com/inconshreveable/mousetrap (spf13#1774)
  docs: add zitadel to the list (spf13#1772)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/flags-args Changes to functionality around command line flags and args size/S Denotes a PR that chanes 10-24 lines
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants