Merge pull request #1525 from urfave/v3-porting

Porting changes from `main` to `v3-dev-main` (circa 2022-10-09)

.github/workflows/cli.yml

@@ -43,9 +43,14 @@ jobs:
           GFLAGS: -tags urfave_cli_no_docs
       - run: make check-binary-size
       - run: make yamlfmt
-      - run: make diffcheck
       - if: matrix.go == '1.19.x' && matrix.os == 'ubuntu-latest'
+        run: make generate
+      - run: make diffcheck
+      # TODO: switch once v3 is released {{
+      # - if: matrix.go == '1.19.x' && matrix.os == 'ubuntu-latest'
+      - if: 'false'
         run: make v2diff
+      # }}
       - if: success() && matrix.go == '1.19.x' && matrix.os == 'ubuntu-latest'
         uses: codecov/codecov-action@v3
         with:

Makefile

@@ -7,7 +7,7 @@
 GO_RUN_BUILD := go run internal/build/build.go
 
 .PHONY: all
-all: generate vet test check-binary-size gfmrun yamlfmt v2diff
+all: generate vet test check-binary-size gfmrun yamlfmt
 
 # NOTE: this is a special catch-all rule to run any of the commands
 # defined in internal/build/build.go with optional arguments passed

app_test.go

@@ -177,7 +177,7 @@ func ExampleApp_Run_commandHelp() {
 	//    greet describeit - use it to see a description
 	//
 	// USAGE:
-	//    greet describeit [command options] [arguments...]
+	//    greet describeit [arguments...]
 	//
 	// DESCRIPTION:
 	//    This is how we describe describeit the function
@@ -956,6 +956,37 @@ func TestApp_UseShortOptionHandlingSubCommand_missing_value(t *testing.T) {
 	expect(t, err, errors.New("flag needs an argument: -n"))
 }
 
+func TestApp_UseShortOptionAfterSliceFlag(t *testing.T) {
+	var one, two bool
+	var name string
+	var sliceValDest StringSlice
+	var sliceVal []string
+	expected := "expectedName"
+
+	app := newTestApp()
+	app.UseShortOptionHandling = true
+	app.Flags = []Flag{
+		&StringSliceFlag{Name: "env", Aliases: []string{"e"}, Destination: &sliceValDest},
+		&BoolFlag{Name: "one", Aliases: []string{"o"}},
+		&BoolFlag{Name: "two", Aliases: []string{"t"}},
+		&StringFlag{Name: "name", Aliases: []string{"n"}},
+	}
+	app.Action = func(c *Context) error {
+		sliceVal = c.StringSlice("env")
+		one = c.Bool("one")
+		two = c.Bool("two")
+		name = c.String("name")
+		return nil
+	}
+
+	_ = app.Run([]string{"", "-e", "foo", "-on", expected})
+	expect(t, sliceVal, []string{"foo"})
+	expect(t, sliceValDest.Value(), []string{"foo"})
+	expect(t, one, true)
+	expect(t, two, false)
+	expect(t, name, expected)
+}
+
 func TestApp_Float64Flag(t *testing.T) {
 	var meters float64
 

cli.go

@@ -22,4 +22,4 @@
 //	}
 package cli
 
-//go:generate go run cmd/urfave-cli-genflags/main.go
+//go:generate make -C cmd/urfave-cli-genflags run

cmd/urfave-cli-genflags/Makefile

@@ -1,6 +1,9 @@
+GOIMPORTS_BIN ?= $(shell which goimports || true)
 GOTEST_FLAGS ?= -v --coverprofile main.coverprofile --covermode count --cover github.com/urfave/cli/v2/cmd/urfave-cli-genflags
 GOBUILD_FLAGS ?= -x
 
+export GOIMPORTS_BIN
+
 .PHONY: all
 all: test build smoke-test
 
@@ -19,3 +22,7 @@ smoke-test: build
 .PHONY: show-cover
 show-cover:
 	go tool cover -func main.coverprofile
+
+.PHONY: run
+run: build
+	./urfave-cli-genflags

cmd/urfave-cli-genflags/generated.gotmpl

@@ -75,16 +75,6 @@ func (f *{{.TypeName}}) TakesValue() bool {
   return "{{.TypeName }}" != "BoolFlag"
 }
 
-{{if .GenerateDefaultText}}
-// GetDefaultText returns the default text for this flag
-func (f *{{.TypeName}}) GetDefaultText() string {
-	if f.DefaultText != "" {
-		return f.DefaultText
-	}
-	return f.GetValue()
-}
-{{end}}
-
 {{end}}{{/* /if .GenerateFlagInterface */}}
 
 {{end}}{{/* /range .SortedFlagTypes */}}

cmd/urfave-cli-genflags/generated_test.gotmpl

@@ -12,13 +12,13 @@ func Test{{.TypeName}}_SatisfiesFlagInterface(t *testing.T) {
 }
 
 func Test{{.TypeName}}_SatisfiesRequiredFlagInterface(t *testing.T) {
-  var f {{$.UrfaveCLITestNamespace}}Flag = &{{$.UrfaveCLITestNamespace}}{{.TypeName}}{}
+  var f {{$.UrfaveCLITestNamespace}}RequiredFlag = &{{$.UrfaveCLITestNamespace}}{{.TypeName}}{}
 
   _ = f.IsRequired()
 }
 
 func Test{{.TypeName}}_SatisfiesVisibleFlagInterface(t *testing.T) {
-  var f {{$.UrfaveCLITestNamespace}}Flag = &{{$.UrfaveCLITestNamespace}}{{.TypeName}}{}
+  var f {{$.UrfaveCLITestNamespace}}VisibleFlag = &{{$.UrfaveCLITestNamespace}}{{.TypeName}}{}
 
   _ = f.IsVisible()
 }

cmd/urfave-cli-genflags/main.go

@@ -4,6 +4,7 @@ import (
 	"bytes"
 	"context"
 	_ "embed"
+	"fmt"
 	"log"
 	"os"
 	"os/exec"
@@ -37,6 +38,9 @@ var (
 func sh(ctx context.Context, exe string, args ...string) (string, error) {
 	cmd := exec.CommandContext(ctx, exe, args...)
 	cmd.Stderr = os.Stderr
+
+	fmt.Fprintf(os.Stderr, "# ---> %s\n", cmd)
+
 	outBytes, err := cmd.Output()
 	return string(outBytes), err
 }
@@ -89,10 +93,19 @@ func main() {
 				Aliases: []string{"N"},
 				Value:   "cli.",
 			},
+			&cli.PathFlag{
+				Name:    "goimports",
+				EnvVars: []string{"GOIMPORTS_BIN"},
+				Value:   filepath.Join(top, ".local/bin/goimports"),
+			},
 		},
 		Action: runGenFlags,
 	}
 
+	if err := os.Chdir(top); err != nil {
+		log.Fatal(err)
+	}
+
 	if err := app.RunContext(ctx, os.Args); err != nil {
 		log.Fatal(err)
 	}
@@ -163,11 +176,11 @@ func runGenFlags(cCtx *cli.Context) error {
 		return err
 	}
 
-	if _, err := sh(cCtx.Context, "goimports", "-w", cCtx.Path("generated-output")); err != nil {
+	if _, err := sh(cCtx.Context, cCtx.Path("goimports"), "-w", cCtx.Path("generated-output")); err != nil {
 		return err
 	}
 
-	if _, err := sh(cCtx.Context, "goimports", "-w", cCtx.Path("generated-test-output")); err != nil {
+	if _, err := sh(cCtx.Context, cCtx.Path("goimports"), "-w", cCtx.Path("generated-test-output")); err != nil {
 		return err
 	}
 

command.go

@@ -295,6 +295,17 @@ func (c *Command) startApp(ctx *Context) error {
 	return app.RunAsSubcommand(ctx)
 }
 
+// VisibleCommands returns a slice of the Commands with Hidden=false
+func (c *Command) VisibleCommands() []*Command {
+	var ret []*Command
+	for _, command := range c.Subcommands {
+		if !command.Hidden {
+			ret = append(ret, command)
+		}
+	}
+	return ret
+}
+
 // VisibleFlagCategories returns a slice containing all the visible flag categories with the flags they contain
 func (c *Command) VisibleFlagCategories() []VisibleFlagCategory {
 	if c.flagCategories == nil {

command_test.go

@@ -422,3 +422,30 @@ func TestCommand_CanAddVFlagOnCommands(t *testing.T) {
 	err := app.Run([]string{"foo", "bar"})
 	expect(t, err, nil)
 }
+
+func TestCommand_VisibleSubcCommands(t *testing.T) {
+
+	subc1 := &Command{
+		Name:  "subc1",
+		Usage: "subc1 command1",
+	}
+	subc3 := &Command{
+		Name:  "subc3",
+		Usage: "subc3 command2",
+	}
+	c := &Command{
+		Name:  "bar",
+		Usage: "this is for testing",
+		Subcommands: []*Command{
+			subc1,
+			{
+				Name:   "subc2",
+				Usage:  "subc2 command2",
+				Hidden: true,
+			},
+			subc3,
+		},
+	}
+
+	expect(t, c.VisibleCommands(), []*Command{subc1, subc3})
+}

flag.go

@@ -135,6 +135,14 @@ type DocGenerationFlag interface {
 	GetEnvVars() []string
 }
 
+// DocGenerationSliceFlag extends DocGenerationFlag for slice-based flags.
+type DocGenerationSliceFlag interface {
+	DocGenerationFlag
+
+	// IsSliceFlag returns true for flags that can be given multiple times.
+	IsSliceFlag() bool
+}
+
 // Countable is an interface to enable detection of flag values which support
 // repetitive flags
 type Countable interface {
@@ -332,24 +340,13 @@ func stringifyFlag(f Flag) string {
 
 	usageWithDefault := strings.TrimSpace(usage + defaultValueString)
 
-	return withEnvHint(df.GetEnvVars(),
-		fmt.Sprintf("%s\t%s", prefixedNames(df.Names(), placeholder), usageWithDefault))
-}
-
-func stringifySliceFlag(usage string, names, defaultVals []string) string {
-	placeholder, usage := unquoteUsage(usage)
-	if placeholder == "" {
-		placeholder = defaultPlaceholder
-	}
-
-	defaultVal := ""
-	if len(defaultVals) > 0 {
-		defaultVal = fmt.Sprintf(formatDefault("%s"), strings.Join(defaultVals, ", "))
+	pn := prefixedNames(df.Names(), placeholder)
+	sliceFlag, ok := f.(DocGenerationSliceFlag)
+	if ok && sliceFlag.IsSliceFlag() {
+		pn = pn + " [ " + pn + " ]"
 	}
 
-	usageWithDefault := strings.TrimSpace(fmt.Sprintf("%s%s", usage, defaultVal))
-	pn := prefixedNames(names, placeholder)
-	return fmt.Sprintf("%s [ %s ]\t%s", pn, pn, usageWithDefault)
+	return withEnvHint(df.GetEnvVars(), fmt.Sprintf("%s\t%s", pn, usageWithDefault))
 }
 
 func hasFlag(flags []Flag, fl Flag) bool {

flag_duration.go

@@ -12,6 +12,14 @@ func (f *DurationFlag) GetValue() string {
 	return f.Value.String()
 }
 
+// GetDefaultText returns the default text for this flag
+func (f *DurationFlag) GetDefaultText() string {
+	if f.DefaultText != "" {
+		return f.DefaultText
+	}
+	return f.GetValue()
+}
+
 // Apply populates the flag given the flag set and environment
 func (f *DurationFlag) Apply(set *flag.FlagSet) error {
 	if val, source, found := flagFromEnvOrFile(f.EnvVars, f.FilePath); found {

flag_float64.go

@@ -12,6 +12,14 @@ func (f *Float64Flag) GetValue() string {
 	return fmt.Sprintf("%v", f.Value)
 }
 
+// GetDefaultText returns the default text for this flag
+func (f *Float64Flag) GetDefaultText() string {
+	if f.DefaultText != "" {
+		return f.DefaultText
+	}
+	return f.GetValue()
+}
+
 // Apply populates the flag given the flag set and environment
 func (f *Float64Flag) Apply(set *flag.FlagSet) error {
 	if val, source, found := flagFromEnvOrFile(f.EnvVars, f.FilePath); found {

flag_float64_slice.go

@@ -83,16 +83,32 @@ func (f *Float64Slice) Get() interface{} {
 // String returns a readable representation of this value
 // (for usage defaults)
 func (f *Float64SliceFlag) String() string {
-	return withEnvHint(f.GetEnvVars(), f.stringify())
+	return FlagStringer(f)
 }
 
 // GetValue returns the flags value as string representation and an empty
 // string if the flag takes no value at all.
 func (f *Float64SliceFlag) GetValue() string {
-	if f.Value != nil {
-		return f.Value.String()
+	var defaultVals []string
+	if f.Value != nil && len(f.Value.Value()) > 0 {
+		for _, i := range f.Value.Value() {
+			defaultVals = append(defaultVals, strings.TrimRight(strings.TrimRight(fmt.Sprintf("%f", i), "0"), "."))
+		}
 	}
-	return ""
+	return strings.Join(defaultVals, ", ")
+}
+
+// GetDefaultText returns the default text for this flag
+func (f *Float64SliceFlag) GetDefaultText() string {
+	if f.DefaultText != "" {
+		return f.DefaultText
+	}
+	return f.GetValue()
+}
+
+// IsSliceFlag implements DocGenerationSliceFlag.
+func (f *Float64SliceFlag) IsSliceFlag() bool {
+	return true
 }
 
 // Apply populates the flag given the flag set and environment
@@ -141,18 +157,6 @@ func (f *Float64SliceFlag) Get(ctx *Context) []float64 {
 	return ctx.Float64Slice(f.Name)
 }
 
-func (f *Float64SliceFlag) stringify() string {
-	var defaultVals []string
-
-	if f.Value != nil && len(f.Value.Value()) > 0 {
-		for _, i := range f.Value.Value() {
-			defaultVals = append(defaultVals, strings.TrimRight(strings.TrimRight(fmt.Sprintf("%f", i), "0"), "."))
-		}
-	}
-
-	return stringifySliceFlag(f.Usage, f.Names(), defaultVals)
-}
-
 // RunAction executes flag action if set
 func (f *Float64SliceFlag) RunAction(c *Context) error {
 	if f.Action != nil {

flag_generic.go

@@ -20,6 +20,14 @@ func (f *GenericFlag) GetValue() string {
 	return ""
 }
 
+// GetDefaultText returns the default text for this flag
+func (f *GenericFlag) GetDefaultText() string {
+	if f.DefaultText != "" {
+		return f.DefaultText
+	}
+	return f.GetValue()
+}
+
 // Apply takes the flagset and calls Set on the generic flag with the value
 // provided by the user for parsing by the flag
 func (f *GenericFlag) Apply(set *flag.FlagSet) error {
@@ -34,6 +42,10 @@ func (f *GenericFlag) Apply(set *flag.FlagSet) error {
 	}
 
 	for _, name := range f.Names() {
+		if f.Destination != nil {
+			set.Var(f.Destination, name, f.Usage)
+			continue
+		}
 		set.Var(f.Value, name, f.Usage)
 	}