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

Ordering of arguments and flags impact one another #1733

Closed
3 tasks done
mntlty opened this issue May 18, 2023 · 3 comments · Fixed by #1987
Closed
3 tasks done

Ordering of arguments and flags impact one another #1733

mntlty opened this issue May 18, 2023 · 3 comments · Fixed by #1987
Labels
area/v2 relates to / is being considered for v2 kind/bug describes or fixes a bug status/triage maintainers still need to look into this

Comments

@mntlty
Copy link

mntlty commented May 18, 2023

My urfave/cli version is

** v2.25.3**

Checklist

  • Are you running the latest v2 release? The list of releases is here.
  • Did you check the manual for your release? The v2 manual is here
  • Did you perform a search about this problem? Here's the GitHub guide about searching.

Dependency Management

  • My project is using go modules.

Describe the bug

Ordering of arguments and flags impact one another, causing a flag value to be set or unset depending on whether it comes before or after an argument. This can result in a silent error where the flag name is recognized but the value isn't set.

To reproduce

Using the flag example from https://cli.urfave.org/v2/examples/flags/ (copying below for ease of reference)

package main

import (
	"fmt"
	"log"
	"os"

	"github.com/urfave/cli/v2"
)

func main() {
	app := &cli.App{
		Flags: []cli.Flag{
			&cli.StringFlag{
				Name:  "lang",
				Value: "english",
				Usage: "language for the greeting",
			},
		},
		Action: func(cCtx *cli.Context) error {
			name := "Nefertiti"
			if cCtx.NArg() > 0 {
				name = cCtx.Args().Get(0)
			}
			if cCtx.String("lang") == "spanish" {
				fmt.Println("Hola", name)
			} else {
				fmt.Println("Hello", name)
			}
			return nil
		},
	}

	if err := app.Run(os.Args); err != nil {
		log.Fatal(err)
	}
}

Observed behavior

# argument
$ ./example my-name
Hello my-name
# flag
$ ./example --lang spanish        
Hola Nefertiti
# flag before argument
$ ./example --lang spanish my-name 
Hola my-name
# argument before flag
$ ./example my-name --lang spanish
Hello my-name

Expected behavior

$ ./example my-name --lang spanish and ./example --lang spanish my-name should both set the --lang flag to the value spanish

Additional context

N/A

Want to fix this yourself?

We'd love to have more contributors on this project! If the fix for
this bug is easily explained and very small, feel free to create a
pull request for it.

Run go version and paste its output here

go version go1.20.4 darwin/arm64

Run go env and paste its output here

# paste `go env` output in here
@mntlty mntlty added area/v2 relates to / is being considered for v2 kind/bug describes or fixes a bug status/triage maintainers still need to look into this labels May 18, 2023
@dearchap
Copy link
Contributor

@mntlty This is by design. This issue has been fixed in v3 wherein a flag can be marked as persistent allowing it to flow through to subcommands. This will not be backported to v2 as v2 is on a feature freeze and maintenance only

@dearchap dearchap closed this as not planned Won't fix, can't repro, duplicate, stale May 21, 2023
@mntlty
Copy link
Author

mntlty commented May 21, 2023

@dearchap in that case, looking forward to V3!

I was confused by the part in your response about subcommands - in the example there are no subcommands, only flags and arguments to defined commands.

@fiatjaf
Copy link

fiatjaf commented Jul 9, 2024

@mntlty please upvote this official feature request here: #1950

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/v2 relates to / is being considered for v2 kind/bug describes or fixes a bug status/triage maintainers still need to look into this
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants