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

Flags are ignored after positional arguments (v4 alpha) #124

Open
antoineco opened this issue Oct 3, 2023 · 10 comments
Open

Flags are ignored after positional arguments (v4 alpha) #124

antoineco opened this issue Oct 3, 2023 · 10 comments

Comments

@antoineco
Copy link

antoineco commented Oct 3, 2023

While experimenting with v4.0.0-alpha.4 for a toy project, I noticed that flags located after any positional argument were ignored during parsing.

The issue can be reproduced with the objectctl example on the latest tag: https://github.com/peterbourgon/ff/tree/v4.0.0-alpha.4/examples/objectctl

Examples:

objectctl create key val --overwrite  # --overwrite is ignored (Overwrite=false)
objectctl create --overwrite key val  # --overwrite is parsed (Overwrite=true)
objectctl create key val --foo  # --foo does not yield an error
objectctl create --foo key val  # --foo yields an error ("unknown flag")
@soniaappasamy
Copy link

Ran into the same today, would be handy to be able to specify the flags either way.

@peterbourgon
Copy link
Owner

objectctl foo --bar baz quux

Is baz a parameter to --bar, or a subcommand of foo?

@antoineco
Copy link
Author

It depends how the bar flag was declared.
If it's a Boolean flag no value should be expected, otherwise the next argument can be expected to be the flag's value without ambiguity. Unless I overlooked an edge case.

@peterbourgon
Copy link
Owner

It depends how the bar flag was declared. If it's a Boolean flag no value should be expected, otherwise the next argument can be expected to be the flag's value without ambiguity. Unless I overlooked an edge case.

What about the case when --bar is a boolean flag, and baz is true, or false, or anything that passes strconv.ParseBool?

Or, consider

objectctl subcommand foo --undefined-flag bar

Do we parse --undefined-flag as a flag and error (because it's not defined)? Or do we parse it as an argument to subcommand like foo and bar?

@antoineco
Copy link
Author

antoineco commented Feb 10, 2024

What about the case when --bar is a boolean flag, and baz is true, or false, or anything that passes strconv.ParseBool?

Fair point, I hadn't noticed that Boolean flags could take a value.


Intuitively I'd expect anything that looks like a flag (-x or --word) to be evaluated as a flag, unless located after a double hyphen (--). It's just the way most Unix tools work and a muscle memory that is hard to undo.

Of course if the library deliberately forces flags to come before any positional argument this is also fine, as long as this design decision is clearly communicated to consumers :)

@antoineco
Copy link
Author

By the way, just to provide more context about what I was originally trying to do:

I have a command with a global flag --verbose/-v which enables verbose logging.
Sometimes, the invocation of a command will fail and return the final error with some context, but not all. When that happens, I typically re-call the same command from my shell history, and append -v before pressing Enter to replay the actions with more details, such as the API calls that are being performed, etc.
With a command implemented with ff, I need instead to travel before the subcommand's positional arguments to add -v.

@peterbourgon
Copy link
Owner

peterbourgon commented Feb 10, 2024

Are you using ff v1/v2/v3, or ff v4?

edit: I see in your OP that you're working with v4, mea culpa

@charbonnierg
Copy link

charbonnierg commented Feb 16, 2024

Hi, I'm using ff v4, and think I hit the same issue.

Below is the code which can be used to reproduce the problem (click to expand):
package main

import (
	"context"
	"errors"
	"fmt"
	"os"

	"github.com/peterbourgon/ff/v4"
	"github.com/peterbourgon/ff/v4/ffhelp"
)

func cli() int {
	root := ff.NewFlagSet("example")
	debug := root.Bool('D', "debug", "enable debug mode")
	exampleCmd := &ff.Command{
		Name:  "example",
		Usage: "example [FLAGS] SUBCOMMAND ...",
		Flags: root,
	}
	// example server
	serverFlags := ff.NewFlagSet("server").SetParent(root)
	serverCmd := &ff.Command{
		Name:      "server",
		Usage:     "example server",
		ShortHelp: "run example server in foreground",
		Flags:     serverFlags,
		Exec: func(ctx context.Context, args []string) error {
			directory := ""
			if len(args) < 1 {
				return fmt.Errorf("missing directory")
			}
			directory = args[0]
			if *debug {
				fmt.Printf("starting server in directory: %s\n", directory)
			}
			return nil
		},
	}
	exampleCmd.Subcommands = append(exampleCmd.Subcommands, serverCmd)
	err := exampleCmd.ParseAndRun(context.Background(), os.Args[1:])
	switch {
	case errors.Is(err, ff.ErrHelp):
		ffhelp.Command(exampleCmd).WriteTo(os.Stderr)
		return 0
	case err != nil:
		fmt.Fprintf(os.Stderr, "error: %v\n", err)
		return 1
	default:
		return 0
	}
}

func main() {
	os.Exit(cli())
}
  • When I run ./example server -D test, it prints the line: starting server in directory: test
  • When I use ./example server test -D it ignores the -D option silently.

I could not understand reading this thread whether this was the intended behavior or not. Do you believe that it should be possible to do that with ff and may work on it in the future or do you believe that ff cannot make choices which will be robust in all cases ?

Anyway thanks this awesome library

@mfridman
Copy link

I decided to kick the tires on v4.0.0-alpha.4, and some of the quality of life improvements we've gone back and forth on have been largely addressed, so thank you for taking the time to rethink parts of v3.

I respect your position, but it'd be great if you could reconsider this particular one.

In practice, the most common thing is being able to mix and match flags and args. If it quaks like a flag, it's registered as a flag, then it's probably a flag (and that's good enough for 99% of cases!).

There will always be some edge cases, and maybe for those situations there could be an escape hatch to "parse as raw" deferring all the logic to the CLI author.

@icio icio mentioned this issue Apr 4, 2024
@peterbourgon
Copy link
Owner

Just to peel back the curtain a bit: I definitely want to make this work. I've been stuck on how to do that without completely re-implementing how flags, flag-sets, args, etc. are parsed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants