Skip to content

Commit

Permalink
Use built-in required flag to zflag + remove formatter
Browse files Browse the repository at this point in the history
  • Loading branch information
hoshsadiq committed Mar 8, 2023
1 parent 19ec3ae commit 25b93fd
Show file tree
Hide file tree
Showing 11 changed files with 37 additions and 124 deletions.
2 changes: 1 addition & 1 deletion .pre-commit-config.yaml
Expand Up @@ -7,7 +7,7 @@ repos:
- id: validate_manifest

- repo: https://github.com/golangci/golangci-lint
rev: v1.47.0
rev: v1.51.2
hooks:
- id: golangci-lint

Expand Down
5 changes: 2 additions & 3 deletions bash_completions.go
Expand Up @@ -10,9 +10,8 @@ import (

// Annotations for Bash completion.
const (
BashCompFilenameExt = "zulu_annotation_bash_completion_filename_extensions"
BashCompOneRequiredFlag = "zulu_annotation_bash_completion_one_required_flag"
BashCompSubdirsInDir = "zulu_annotation_bash_completion_subdirs_in_dir"
BashCompFilenameExt = "zulu_annotation_bash_completion_filename_extensions"
BashCompSubdirsInDir = "zulu_annotation_bash_completion_subdirs_in_dir"
)

func nonCompletableFlag(flag *zflag.Flag) bool {
Expand Down
44 changes: 6 additions & 38 deletions command.go
Expand Up @@ -863,7 +863,7 @@ func (c *Command) CancelRun() {

func (c *Command) execute(a []string) (err error) {
if c == nil {
return fmt.Errorf("Called Execute() on a nil Command")
return fmt.Errorf("called Execute() on a nil Command")
}

if len(c.Deprecated) > 0 {
Expand Down Expand Up @@ -952,10 +952,12 @@ func (c *Command) execute(a []string) (err error) {
})

hooks = append(hooks, func(cmd *Command, args []string) error {
argWoFlags = c.Flags().Args()
if c.DisableFlagParsing {
argWoFlags = a
return nil
}

argWoFlags = c.Flags().Args()
return nil
})

Expand All @@ -973,16 +975,11 @@ func (c *Command) execute(a []string) (err error) {

prependHooks(&hooks, c.preRunHooks, c.PreRunE)

// Include the validateRequiredFlags() logic as a hook
// Include the validateFlagGroups() logic as a hook
// to be executed before running the main Run hooks.
hooks = append(hooks, func(cmd *Command, args []string) error {
err := cmd.validateRequiredFlags()
if err != nil {
return c.FlagErrorFunc()(c, err)
}

if err := c.validateFlagGroups(); err != nil {
return err
return c.FlagErrorFunc()(c, err)
}

return nil
Expand Down Expand Up @@ -1198,30 +1195,6 @@ func (c *Command) ValidateArgs(args []string) error {
return c.Args(c, args)
}

func (c *Command) validateRequiredFlags() error {
if c.DisableFlagParsing {
return nil
}

flags := c.Flags()
var missingFlagNames []string
flags.VisitAll(func(pflag *zflag.Flag) {
requiredAnnotation, found := pflag.Annotations[BashCompOneRequiredFlag]
if !found {
return
}
if requiredAnnotation[0] == "true" && !pflag.Changed {
missingFlagNames = append(missingFlagNames, pflag.Name)
}
})

if len(missingFlagNames) > 0 {
return fmt.Errorf(`required flag(s) "%s" not set`, strings.Join(missingFlagNames, `", "`))
}

return nil
}

// InitDefaultHelpFlag adds default help flag to c.
// It is called automatically by executing the c or by calling help and usage.
// If c already has help flag, it will do nothing.
Expand Down Expand Up @@ -1681,7 +1654,6 @@ func (c *Command) Flags() *zflag.FlagSet {
c.flagErrorBuf = new(bytes.Buffer)
}
c.flags.SetOutput(c.flagErrorBuf)
c.flags.FlagUsageFormatter = defaultUsageFormatter
}

return c.flags
Expand Down Expand Up @@ -1710,7 +1682,6 @@ func (c *Command) LocalFlags() *zflag.FlagSet {
c.flagErrorBuf = new(bytes.Buffer)
}
c.lflags.SetOutput(c.flagErrorBuf)
c.lflags.FlagUsageFormatter = defaultUsageFormatter
}

c.lflags.SortFlags = c.Flags().SortFlags
Expand Down Expand Up @@ -1739,7 +1710,6 @@ func (c *Command) InheritedFlags() *zflag.FlagSet {
c.flagErrorBuf = new(bytes.Buffer)
}
c.iflags.SetOutput(c.flagErrorBuf)
c.iflags.FlagUsageFormatter = defaultUsageFormatter
}

local := c.LocalFlags()
Expand Down Expand Up @@ -1768,7 +1738,6 @@ func (c *Command) PersistentFlags() *zflag.FlagSet {
c.flagErrorBuf = new(bytes.Buffer)
}
c.pflags.SetOutput(c.flagErrorBuf)
c.pflags.FlagUsageFormatter = defaultUsageFormatter
}
return c.pflags
}
Expand Down Expand Up @@ -1899,7 +1868,6 @@ func (c *Command) updateParentsPflags() {
c.parentsPflags = zflag.NewFlagSet(c.Name(), zflag.ContinueOnError)
c.parentsPflags.SetOutput(c.flagErrorBuf)
c.parentsPflags.SortFlags = false
c.parentsPflags.FlagUsageFormatter = defaultUsageFormatter
}

if c.globNormFunc != nil {
Expand Down
29 changes: 15 additions & 14 deletions command_test.go
Expand Up @@ -60,6 +60,7 @@ func resetCommandLineFlagSet() {
}

func checkStringContains(t *testing.T, got, expected string) {
t.Helper()
if !strings.Contains(got, expected) {
t.Errorf("Expected to contain: \n %v\nGot:\n %v\n", expected, got)
}
Expand Down Expand Up @@ -777,10 +778,10 @@ func TestPersistentFlagsOnChild(t *testing.T) {

func TestRequiredFlags(t *testing.T) {
c := &zulu.Command{Use: "c", RunE: noopRun}
c.Flags().String("foo1", "", "", zulu.FlagOptRequired())
c.Flags().String("foo2", "", "", zulu.FlagOptRequired())
c.Flags().String("foo1", "", "", zflag.OptRequired())
c.Flags().String("foo2", "", "", zflag.OptRequired())
c.Flags().String("bar", "", "")
expected := fmt.Sprintf("required flag(s) %q, %q not set", "foo1", "foo2")
expected := fmt.Sprintf("required flag(s) %q, %q not set", "--foo1", "--foo2")

_, err := executeCommand(c)
got := err.Error()
Expand All @@ -792,14 +793,14 @@ func TestRequiredFlags(t *testing.T) {

func TestRequiredFlagsWithCustomFlagErrorFunc(t *testing.T) {
c := &zulu.Command{Use: "c", RunE: noopRun}
c.Flags().String("foo1", "", "", zulu.FlagOptRequired())
c.Flags().String("foo1", "", "", zflag.OptRequired())
silentError := "failed flag parsing"
c.SetFlagErrorFunc(func(c *zulu.Command, err error) error {
c.Println(err)
c.Println(c.UsageString())
return errors.New(silentError)
})
requiredFlagErrorMessage := fmt.Sprintf("required flag(s) %q not set", "foo1")
requiredFlagErrorMessage := fmt.Sprintf("required flag(s) %q not set", "--foo1")

output, err := executeCommand(c)
got := err.Error()
Expand All @@ -813,18 +814,18 @@ func TestRequiredFlagsWithCustomFlagErrorFunc(t *testing.T) {

func TestPersistentRequiredFlags(t *testing.T) {
parent := &zulu.Command{Use: "parent", RunE: noopRun}
parent.PersistentFlags().String("foo1", "", "", zulu.FlagOptRequired())
parent.PersistentFlags().String("foo2", "", "", zulu.FlagOptRequired())
parent.PersistentFlags().String("foo1", "", "", zflag.OptRequired())
parent.PersistentFlags().String("foo2", "", "", zflag.OptRequired())
parent.Flags().String("foo3", "", "")

child := &zulu.Command{Use: "child", RunE: noopRun}
child.Flags().String("bar1", "", "", zulu.FlagOptRequired())
child.Flags().String("bar2", "", "", zulu.FlagOptRequired())
child.Flags().String("bar1", "", "", zflag.OptRequired())
child.Flags().String("bar2", "", "", zflag.OptRequired())
child.Flags().String("bar3", "", "")

parent.AddCommand(child)

expected := fmt.Sprintf("required flag(s) %q, %q, %q, %q not set", "bar1", "bar2", "foo1", "foo2")
expected := fmt.Sprintf("required flag(s) %q, %q, %q, %q not set", "--bar1", "--bar2", "--foo1", "--foo2")

_, err := executeCommand(parent, "child")
if err.Error() != expected {
Expand All @@ -837,7 +838,7 @@ func TestPersistentRequiredFlagsWithDisableFlagParsing(t *testing.T) {
// commands that disable flag parsing

parent := &zulu.Command{Use: "parent", RunE: noopRun}
parent.PersistentFlags().Bool("foo", false, "", zulu.FlagOptRequired())
parent.PersistentFlags().Bool("foo", false, "", zflag.OptRequired())
flag := parent.PersistentFlags().Lookup("foo")

child := &zulu.Command{Use: "child", RunE: noopRun}
Expand Down Expand Up @@ -2463,18 +2464,18 @@ Use "root child [command] --help" for more information about a command.
child.Example = "child sub --int 0"

pfs := root.PersistentFlags()
pfs.Int("pint", 1, "persistent int usage", zflag.OptShorthand('q'), zflag.OptGroup("group1"), zulu.FlagOptRequired())
pfs.Int("pint", 1, "persistent int usage", zflag.OptShorthand('q'), zflag.OptGroup("group1"), zflag.OptRequired())
pfs.Bool("pbool", false, "persistent bool usage", zflag.OptShorthand('c'), zflag.OptGroup("group2"))

fs := child.Flags()
fs.String("string1", "some", "string1 usage", zflag.OptShorthand('s'))
fs.Bool("bool1", false, "bool1 usage", zflag.OptShorthand('b'))

fs.String("string2", "some", "string2 usage in group1", zflag.OptGroup("group1"), zulu.FlagOptRequired())
fs.String("string2", "some", "string2 usage in group1", zflag.OptGroup("group1"), zflag.OptRequired())
fs.Bool("bool2", false, "bool2 usage in group1", zflag.OptGroup("group1"))

fs.String("string3", "some", "string3 usage in group2", zflag.OptGroup("group2"))
fs.Bool("bool3", false, "bool3 usage in group2", zflag.OptGroup("group2"), zulu.FlagOptRequired())
fs.Bool("bool3", false, "bool3 usage in group2", zflag.OptGroup("group2"), zflag.OptRequired())

sub1 := &zulu.Command{
Use: "sub1",
Expand Down
9 changes: 4 additions & 5 deletions completions.go
Expand Up @@ -255,6 +255,7 @@ func (c *Command) getCompletions(args []string) (*Command, []string, ShellCompDi
// Let's add the --help and --version flag ourselves.
finalCmd.InitDefaultHelpFlag()
finalCmd.InitDefaultVersionFlag()
finalCmd.FParseErrAllowList.RequiredFlags = true

// Check if we are doing flag value completion before parsing the flags.
// This is important because if we are completing a flag value, we need to also
Expand Down Expand Up @@ -512,11 +513,9 @@ func completeRequireFlags(finalCmd *Command, toComplete string) []string {
var completions []string

doCompleteRequiredFlags := func(flag *zflag.Flag) {
if _, present := flag.Annotations[BashCompOneRequiredFlag]; present {
if !flag.Changed {
// If the flag is not already present, we suggest it as a completion
completions = append(completions, getFlagNameCompletions(flag, toComplete)...)
}
if flag.Required && !flag.Changed {
// If the flag is not already present, we suggest it as a completion
completions = append(completions, getFlagNameCompletions(flag, toComplete)...)
}
}

Expand Down
10 changes: 5 additions & 5 deletions completions_test.go
Expand Up @@ -10,7 +10,7 @@ import (
"github.com/zulucmd/zulu"
)

func validArgsFunc(cmd *zulu.Command, args []string, toComplete string) ([]string, zulu.ShellCompDirective) {
func validArgsFunc(_ *zulu.Command, args []string, toComplete string) ([]string, zulu.ShellCompDirective) {
if len(args) != 0 {
return nil, zulu.ShellCompDirectiveNoFileComp
}
Expand Down Expand Up @@ -798,15 +798,15 @@ func TestRequiredFlagNameCompletionInGo(t *testing.T) {
}
rootCmd.AddCommand(childCmd)

rootCmd.Flags().Int("requiredFlag", -1, "required flag", zflag.OptShorthand('r'), zulu.FlagOptRequired())
rootCmd.Flags().Int("requiredFlag", -1, "required flag", zflag.OptShorthand('r'), zflag.OptRequired())
requiredFlag := rootCmd.Flags().Lookup("requiredFlag")

rootCmd.PersistentFlags().Int("requiredPersistent", -1, "required persistent", zflag.OptShorthand('p'), zulu.FlagOptRequired())
rootCmd.PersistentFlags().Int("requiredPersistent", -1, "required persistent", zflag.OptShorthand('p'), zflag.OptRequired())
requiredPersistent := rootCmd.PersistentFlags().Lookup("requiredPersistent")

rootCmd.Flags().String("release", "", "Release name", zflag.OptShorthand('R'))

childCmd.Flags().Bool("subRequired", false, "sub required flag", zflag.OptShorthand('s'), zulu.FlagOptRequired())
childCmd.Flags().Bool("subRequired", false, "sub required flag", zflag.OptShorthand('s'), zflag.OptRequired())
childCmd.Flags().Bool("subNotRequired", false, "sub not required flag", zflag.OptShorthand('n'))

// Test that a required flag is suggested even without the - prefix
Expand Down Expand Up @@ -2886,7 +2886,7 @@ func TestCompletionCobraFlags(t *testing.T) {

rootCmd.AddCommand(childCmd, childCmd2, childCmd3)

_ = childCmd.Flags().Bool("bool", false, "A bool flag", zulu.FlagOptRequired())
_ = childCmd.Flags().Bool("bool", false, "A bool flag", zflag.OptRequired())

// Have a command that adds its own help and version flag
_ = childCmd2.Flags().Bool("help", false, "My own help", zflag.OptShorthand('h'))
Expand Down
2 changes: 1 addition & 1 deletion flag_groups.go
Expand Up @@ -96,7 +96,7 @@ func (g *requiredTogetherFlagGroup) AdjustCommandForCompletions(c *Command) {
if setFlags.hasAnyFrom(g.flagNames) {
for _, requiredFlagName := range g.flagNames {
f := c.Flags().Lookup(requiredFlagName)
_ = FlagOptRequired()(f)
_ = zflag.OptRequired()(f)
}
}
}
Expand Down
47 changes: 0 additions & 47 deletions formatter.go

This file was deleted.

2 changes: 1 addition & 1 deletion go.mod
Expand Up @@ -5,6 +5,6 @@ go 1.16
require (
github.com/cpuguy83/go-md2man/v2 v2.0.2
github.com/inconshreveable/mousetrap v1.1.0
github.com/zulucmd/zflag v1.0.0
github.com/zulucmd/zflag v1.1.2
gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c
)
4 changes: 2 additions & 2 deletions go.sum
Expand Up @@ -4,8 +4,8 @@ github.com/inconshreveable/mousetrap v1.1.0 h1:wN+x4NVGpMsO7ErUn/mUI3vEoE6Jt13X2
github.com/inconshreveable/mousetrap v1.1.0/go.mod h1:vpF70FUmC8bwa3OWnCshd2FqLfsEA9PFc4w1p2J65bw=
github.com/russross/blackfriday/v2 v2.1.0 h1:JIOH55/0cWyOuilr9/qlrm0BSXldqnqwMsf35Ld67mk=
github.com/russross/blackfriday/v2 v2.1.0/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM=
github.com/zulucmd/zflag v1.0.0 h1:viByzLR5dTrB7KFytbBQ8qU/8Z27bvtMU3A4rcuqZtU=
github.com/zulucmd/zflag v1.0.0/go.mod h1:snFF93KDcScS6xPJfXMGW5N5X/BON28FuZRUT/ulR6g=
github.com/zulucmd/zflag v1.1.2 h1:b2uVECG0WtBqBeQAwNizh9ACznU2yISneMNumUvkId4=
github.com/zulucmd/zflag v1.1.2/go.mod h1:snFF93KDcScS6xPJfXMGW5N5X/BON28FuZRUT/ulR6g=
gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM=
gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c h1:dUUwHk2QECo/6vqA44rthZ8ie2QXMNeKRTHCNY2nXvo=
Expand Down
7 changes: 0 additions & 7 deletions shell_completions.go
Expand Up @@ -6,13 +6,6 @@ import (
"github.com/zulucmd/zflag"
)

// FlagOptRequired instructs the various shell completion implementations to
// prioritize the flag when performing completion, and causes your command
// to report an error if invoked without the flag.
func FlagOptRequired() zflag.Opt {
return zflag.OptAnnotation(BashCompOneRequiredFlag, []string{"true"})
}

// FlagOptFilename instructs the various shell completion implementations to
// limit completions for the flag to the specified file extensions.
func FlagOptFilename(extensions ...string) zflag.Opt {
Expand Down

0 comments on commit 25b93fd

Please sign in to comment.