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.

Sign up for GitHub

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

Jump to bottom

feat: add ignore command option #144

Merged
merged 3 commits into from Jul 7, 2021
Merged

feat: add ignore command option #144

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ffaa9da
feat: add ignore command option
ankithans Jul 6, 2021
4b82b8f
feat: implement skip command options
ankithans Jul 7, 2021
49d4f6c
add reasonable examples
wtrocki Jul 7, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 0 additions & 1 deletion validator/example/cmd.go
View file
Open in desktop
Expand Up @@ -18,7 +18,6 @@ func NewCommand() *cobra.Command {

cmd := &cobra.Command{
Use: "cmd0",
Short: "This is the short",
Long: `Lorem ipsum dolor sit amet, consectetur adipiscing elit. Duis malesuada varius lacus, sit amet dictum risus convallis nec. Quisque suscipit at neque in blandit. Proin a accumsan ante. Aenean cursus suscipit sem. Nunc sollicitudin, ante et vehicula pharetra, mauris elit porta felis, et ultricies nulla justo eleifend justo. Proin sit amet.`,
Example: "Lorem ipsum dolor sit amet, consectetur adipiscing elit. Duis malesuada varius lacus, sit amet dictum risus convallis nec. Quisque suscipit at neque in blandit. Proin a accumsan ante. Aenean cursus suscipit sem. Nunc sollicitudin, ante et vehicula pharetra, mauris elit porta felis, et ultricies nulla justo eleifend justo. Proin sit amet.",
Run: func(cmd *cobra.Command, args []string) {
Expand Down
3 changes: 3 additions & 0 deletions validator/example/cmd_test.go
View file
Open in desktop
Expand Up @@ -12,6 +12,9 @@ func Test_ExecuteCommand(t *testing.T) {
// Testing cobra commands with default recommended config
// default config can also be overrided
ruleCfg := rules.ValidatorConfig{
ValidatorOptions: rules.ValidatorOptions{
IgnoreCommands: map[string]bool{"cmd0": true},
},
ValidatorRules: rules.ValidatorRules{
Length: rules.Length{
Limits: map[string]rules.Limit{
Expand Down
3 changes: 2 additions & 1 deletion validator/rules/config.go
View file
Open in desktop
Expand Up @@ -17,7 +17,8 @@ type ValidatorConfig struct {
// ValidatorOptions provide additional configurations
// to the rules
type ValidatorOptions struct {
Verbose bool `json:"Verbose"`
Verbose bool `json:"Verbose"`
IgnoreCommands map[string]bool `json:"IgnoreCommands"`
}

// ValidatorRules consists of all the rules
Expand Down
27 changes: 17 additions & 10 deletions validator/rules/executor.go
View file
Open in desktop
Expand Up @@ -30,13 +30,13 @@ func ExecuteRulesInternal(cmd *cobra.Command, ruleConfig *RuleConfig, userValida
initDefaultRules(userValidatorConfig, ruleConfig)

// validate the root command
validate(cmd, &info, ruleConfig)
validate(cmd, &info, ruleConfig, userValidatorConfig)

return executeHelper(cmd, &info, ruleConfig)
return executeHelper(cmd, &info, ruleConfig, userValidatorConfig)
}

func executeHelper(cmd *cobra.Command, info *validator.StatusLog, ruleConfig *RuleConfig) []validator.ValidationError {
info.Errors = executeRecursive(cmd, info, ruleConfig)
func executeHelper(cmd *cobra.Command, info *validator.StatusLog, ruleConfig *RuleConfig, userValidatorConfig *ValidatorConfig) []validator.ValidationError {
info.Errors = executeRecursive(cmd, info, ruleConfig, userValidatorConfig)

// prints additional info for the checks
fmt.Fprintf(os.Stderr, "commands checked: %d\nchecks failed: %d\n", info.TotalTested, info.TotalErrors)
Expand All @@ -46,35 +46,42 @@ func executeHelper(cmd *cobra.Command, info *validator.StatusLog, ruleConfig *Ru

// executeRecursive recursively traverse over all the subcommands
// and validate using executeRulesChildren function
func executeRecursive(cmd *cobra.Command, info *validator.StatusLog, ruleConfig *RuleConfig) []validator.ValidationError {
func executeRecursive(cmd *cobra.Command, info *validator.StatusLog, ruleConfig *RuleConfig, userValidatorConfig *ValidatorConfig) []validator.ValidationError {
for _, child := range cmd.Commands() {
// base case
if !child.IsAvailableCommand() || child.IsAdditionalHelpTopicCommand() {
continue
}
// recursive call
info.Errors = executeRecursive(child, info, ruleConfig)
info.Errors = executeRecursive(child, info, ruleConfig, userValidatorConfig)
}
info.Errors = executeRulesChildren(cmd, info, ruleConfig)
info.Errors = executeRulesChildren(cmd, info, ruleConfig, userValidatorConfig)

return info.Errors
}

// executeRulesChildren execute rules on children of cmd
func executeRulesChildren(cmd *cobra.Command, info *validator.StatusLog, ruleConfig *RuleConfig) []validator.ValidationError {
func executeRulesChildren(cmd *cobra.Command, info *validator.StatusLog, ruleConfig *RuleConfig, userValidatorConfig *ValidatorConfig) []validator.ValidationError {
children := cmd.Commands()
for _, child := range children {

if !child.IsAvailableCommand() || child.IsAdditionalHelpTopicCommand() {
continue
}
validate(child, info, ruleConfig)
validate(child, info, ruleConfig, userValidatorConfig)
}
return info.Errors
}

// validate returns validation errors by executing the rules
func validate(cmd *cobra.Command, info *validator.StatusLog, ruleConfig *RuleConfig) {
func validate(cmd *cobra.Command, info *validator.StatusLog, ruleConfig *RuleConfig, userValidatorConfig *ValidatorConfig) {

// if command needs to be ignored
if val, ok := userValidatorConfig.ValidatorOptions.IgnoreCommands[cmd.Use]; ok {
Copy link
Contributor

Choose a reason for hiding this comment

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

How this will work with nested commands? Do we use . to map the nested commands.

Example:

kafka.create 
registry.create

Copy link
Member Author

Choose a reason for hiding this comment

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

it captures Use of the command, so

kafka.create 
registry.create

all create subcommands will be ignored. Let's fix this!

Copy link
Contributor

@wtrocki wtrocki Jul 6, 2021 •
edited

Choose a reason for hiding this comment

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

However, would that be hard? Can we simply create method that returns true or false for the current command and rule rule?

EDIT:

Ignore me that will be bad idea.

I think we might need something as follows:

  • Ignore entire chain of commands (when you do dev etc.) and still want tests to pass
  • Ignore single command in nested scenario for all rules
  • Ignore single command in nested scenario for some rule.

Instead of impleentatation I would love to see config you will come up with so we can discuss it.
Remeber - we focus on API and docs first :D

Copy link
Member Author

@ankithans ankithans Jul 6, 2021 •
edited

Choose a reason for hiding this comment

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

for nested/all commands, best way is to use CommandPath() provided by cobra.
user will specify : {"kafka create": true, "echo": true}

This config is coming to my mind atm -

type ValidatorOptions struct {
	Verbose      bool            
	SkipChildren map[string]bool 
	SkipCommands map[string]bool 
}

RuleOptions to be present in each rule, for controlling rule specific properties/options

type RuleOptions struct {
	Verbose      bool
	SkipCommands map[string]bool
}

eg -

type Length struct {
	RuleOptions validator.RuleOptions
	Limits      map[string]Limit
}

Copy link
Contributor

@wtrocki wtrocki Jul 6, 2021 •
edited

Choose a reason for hiding this comment

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

type Length struct {
	RuleOptions validator.RuleOptions

Could that be part of the interface so we can still have generic processing and enforce options for each rule

SkipChildren map[string]bool

Not needed/confusing. If you specify command path that is groping other commands all grouped ones will be disabled.

 map[string]bool

Not sure if that would give us enough to control command (path to command), rule, level - Error/Ignore (could be warn in the future)

Copy link
Member Author

Choose a reason for hiding this comment

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

RuleOptions validator.RuleOptions
Could that be part of the interface so we can still have generic processing and enforce options for each rule

Rules interface?

SkipChildren map[string]bool
Not needed/confusing. If you specify command path that is grouping other commands all grouped ones will be disabled.

can you please provide an example for understanding

map[string]bool
Not sure if that would give us enough to control command (path to command), rule, level - Error/Ignore (could be warn in the future)

do we need all this details, when we ignore some command?

Copy link
Contributor

Choose a reason for hiding this comment

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

do we need all this details, when we ignore some command?

Agreed. No need

if val {
return
}
}

// traverse all rules and validate
for _, rule := range ruleConfig.Rules {
Expand Down
10 changes: 10 additions & 0 deletions website/docs/validator.md
View file
Open in desktop
Expand Up @@ -42,4 +42,14 @@ for _, errs := range validationErr {
t.Errorf("%s: cmd %s: %s", errs.Rule, errs.Cmd.CommandPath(), errs.Name)
}
}
```

## Ignore Commands
Validation for selected commands can be ignored, by passing the `Use` of commands to be skipped in `IgnoreCommands` attribute in ValidatorOptions
```go
ruleCfg := rules.ValidatorConfig{
ValidatorOptions: rules.ValidatorOptions{
IgnoreCommands: map[string]bool{"echo": true},
},
}
```