Skip to content

Conversation

jinlohwu
Copy link
Contributor

@jinlohwu jinlohwu 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.

@jinlohwu
Copy link
Contributor Author

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.

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

@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