cobra
cobra copied to clipboard
feat: add markflagsdependenton (#1739)
This PR adds to the group validation capability that came in the Spring release with the advent of MarkFlagsRequiredTogether
and MarkFlagsMutuallyExclusive
by adding 2 further group validation options (new methods on the cobra.Command):
- MarkFlagsDependsOn: 1 or more flags are dependentent upon another
- MarkFlagDependsOnAny: a flag depends on at least 1 of a group of other flags
These 2 options differ from MarkFlagsRequiredTogether
and MarkFlagsMutuallyExclusive
in a key aspect, that being that the relationship is 1 way, ie:
- for MarkFlagsDependsOn: Flags A and B depends on Flag X, but X is not dependent on A or B.
- for MarkFlagDependsOnAny: A is dependent at least 1 of X or Y, but X and Y ore not dependent on A.
I have tried to minimise the amount of code changes in this PR. If I had free will, I would have made all the code consistent, but that would have resulted in more code changes. If you would prefer for me to make existing code consistent with new, then please let me know and I will update.
More detailed explaination of changes:
-
setting annotations
: 2 new unexported functions has been defined markAnnotation and setFlagAnnotation which mimmicks how annotations are added for MarkFlagsRequiredTogether and MarkFlagsMutuallyExclusive (referred to as non special flag validators). For those 2 non special validators, code is duplicated and the only difference between the 2 implemenations is the error message that is defined for the panic, if the flag can't be found. markAnnotation/setFlagAnnotation avoid the duplication, by using a format parameter. Ideally, the non special validators should be updated to use markAnnotation, but I avoided this to reduce on code churn. -
special status
: for the non special validators, all parameters in the group have identical status. The new special validators (MarkFlagsDependsOn and MarkFlagDependsOnAny) impose a different status to the first flag in the group. For MarkFlagsDependsOn, the first flag is thedependee
, that all other flags in the group depend on. For MarkFlagDependsOnAny, the first flag depends on at least one of the other flags being present.
More on special status
: To support this, a new collection specialGroupInfoCollection and associated structs specialStatusInfoData and specialGroupInfo have been defined.
The non special valdators use processFlagForGroupAnnotation which internally defines a map of string to bool, indicating whether a flag is present or not. For the new special validators, this is not enough. For a particluar group, we need to distinguish which flag in the group is the special one. So:
- specialGroupInfoCollection: maps the group name to a specialGroupInfo
-
specialGroupInfo: contains
special
which indicates which is the special flag,others
the remaing flags. To maintain similarilty with existing code, we also have a member of type specialStatusInfoData - specialStatusInfoData: simply maps a flag name to a boolean value equivalent to isSet
Defined 2 new group validator functions: validateDependsOnFlagGroups and validateDependsOnAnyFlagGroups, both of which use the newly defined struct specialGroupInfoCollection for validation which are equivalent to the valitdators that exist for non special group validators validateRequiredFlagGroups and validateExclusiveFlagGroups.
TODO: still need completions tests for MarkFlagsDependsOn. There is no completions functionality for MarkFlagDependsOnAny, because if flag A
is dependent on X
or Y
, we can't mark either as required using MarkFlagRequired and we can't hide either of them by setting the Hidden property to true.
This PR exceeds the recommended size of 200 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR might be rejected due to its size.
PS: I was not aware of size limitations on PRs. There is no extraneous code, it is all focused on a single issue. Some of the changes are due to formatting so they are not real changes, and the other contribution of size comes from a comprehensive description in the user guide. So the github-actions bot is to some extent uneccessarily crying wolf 🐺
This PR exceeds the recommended size of 200 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR might be rejected due to its size.
Thanks @plastikfan for the contribution! Don't worry about the size of the PR if it is justified. Please be patient with us as we'll do our best to review this as soon as we can.
Thanks @marckhouzam, I was wondering wether I should continue with this PR because of that message. I am just writing completions tests for this as it is something that slipped my mind until I reviewed my code for the PR.
This PR exceeds the recommended size of 200 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR might be rejected due to its size.
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.
Just now waiting the outcome of #1775, as that PR claims to implement the 2 new validation groups I have proposed as part of its own design refactor. Because of this, #1739 may be redundant.
Closing this PR as it being superceded by #1775