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.

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

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

Merged
merged 25 commits into from Oct 16, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
a6f5ca6
wrap: Avoid trailing whitespace for empty lines
abitrolly Oct 3, 2022
d3bb381
Fix test for removed trailing whitespace
abitrolly Oct 3, 2022
05fb755
wrap: Simplify loop logic
abitrolly Oct 4, 2022
e14dca7
Run `go fmt`
abitrolly Oct 4, 2022
4959a9f
Refactor `wrap()` and add test for empty line
abitrolly Oct 4, 2022
82ea9f7
Call FlagStringer in String() method of slice flags
fjl Sep 30, 2022
65c98c8
Ensure "generate" step runs in CI prior to diff check
meatballhat Sep 29, 2022
c5057d1
Only run generate on go 1.19
meatballhat Sep 30, 2022
01bdec7
Set destination in GenericFlag apply function
nkuba Jul 26, 2022
b82e628
Add unit test for GenericFlag Destination parsing
nkuba Jul 26, 2022
13860a7
Fix:(issue_1505) Fix flag alignment in help
dearchap Sep 30, 2022
ae8c511
Fix command help subcommand
dearchap Sep 30, 2022
c86805d
Add test case
dearchap Sep 30, 2022
dccd762
Componentize template
dearchap Sep 30, 2022
a4b7759
Update template.go
dearchap Oct 5, 2022
9a94619
Add test coverage for Command.VisibleCommands()
dearchap Oct 5, 2022
ea28930
Remove extra 3 spaces in last line
dearchap Oct 5, 2022
924ebda
Fix:(issue_1500). Fix slice flag value duplication issue
dearchap Sep 29, 2022
fcb0bce
Fix failed test
dearchap Oct 5, 2022
64facdb
Merge remote-tracking branch 'origin/v3-dev-main' into v3-porting
meatballhat Oct 9, 2022
9621675
Remove duplicate DocGenerationFlag interface
meatballhat Oct 9, 2022
6404f1d
Backfill drop of generated GetDefaultText method
meatballhat Oct 10, 2022
b458207
Build and run `urfave-cli-genflags` via its `Makefile`
meatballhat Oct 9, 2022
75aabac
Use existing goimports installation if available
meatballhat Oct 10, 2022
85ff0c5
Un-regress from v3 porting losses
meatballhat Oct 13, 2022
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
7 changes: 6 additions & 1 deletion .github/workflows/cli.yml
Expand Up @@ -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:
Expand Down
2 changes: 1 addition & 1 deletion Makefile
Expand Up @@ -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
Expand Down
33 changes: 32 additions & 1 deletion app_test.go
Expand Up @@ -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
Expand Down Expand Up @@ -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

Expand Down
2 changes: 1 addition & 1 deletion cli.go
Expand Up @@ -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
7 changes: 7 additions & 0 deletions 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

Expand All @@ -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
10 changes: 0 additions & 10 deletions cmd/urfave-cli-genflags/generated.gotmpl
Expand Up @@ -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 */}}
Expand Down
4 changes: 2 additions & 2 deletions cmd/urfave-cli-genflags/generated_test.gotmpl
Expand Up @@ -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()
}
Expand Down
17 changes: 15 additions & 2 deletions cmd/urfave-cli-genflags/main.go
Expand Up @@ -4,6 +4,7 @@ import (
"bytes"
"context"
_ "embed"
"fmt"
"log"
"os"
"os/exec"
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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
}

Expand Down
11 changes: 11 additions & 0 deletions command.go
Expand Up @@ -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 {
Expand Down
27 changes: 27 additions & 0 deletions command_test.go
Expand Up @@ -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})
}
29 changes: 13 additions & 16 deletions flag.go
Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down
8 changes: 8 additions & 0 deletions flag_duration.go
Expand Up @@ -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 {
Expand Down
8 changes: 8 additions & 0 deletions flag_float64.go
Expand Up @@ -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 {
Expand Down
36 changes: 20 additions & 16 deletions flag_float64_slice.go
Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand Down
12 changes: 12 additions & 0 deletions flag_generic.go
Expand Up @@ -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 {
Expand All @@ -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)
}

Expand Down