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

Jump to bottom

proposal: flag: support comma-separated flag aliasing #35761

Closed
turtletowerz opened this issue Nov 21, 2019 · 9 comments
Closed

proposal: flag: support comma-separated flag aliasing #35761

turtletowerz opened this issue Nov 21, 2019 · 9 comments
Labels
FrozenDueToAge Proposal
Projects
Proposals (old)
Milestone
Proposal

Comments

@turtletowerz
Copy link

turtletowerz commented Nov 21, 2019 •
edited

One trait a majority of popular command-line applications have is their support for shorthand notation of flags. For example, ffmpeg allows for the use of -c, which is shorthand representation of the -codec flag. The user can choose between a shorter flag or one that is more clear/well-defined

Currently with the flag package this is not possible, and instead a flag must be defined twice with essentially the same parameters. The best example of this inefficient process is in the /src/flag/example_test.go file, where the gopherType variable is assigned two different flags with code that (more or less) is the same.

// Example 2: Two flags sharing a variable, so we can have a shorthand.
// The order of initialization is undefined, so make sure both use the
// same default value. They must be set up with an init function.
var gopherType string

func init() {
	const (
		defaultGopher = "pocket"
		usage         = "the variety of gopher"
	)
	flag.StringVar(&gopherType, "gopher_type", defaultGopher, usage)
	flag.StringVar(&gopherType, "g", defaultGopher, usage+" (shorthand)")
}

My proposal is that flagset.Var is modified to support a name string that can contain several flag names separated by commas. The function would split the string by commas, and then range over the results and trim the white-space from them using strings.TrimSpace. You could then place the rest of the flagset.Var function in this range loop and it would assign a flag for every comma-separated flag name found. I created a non-functional example below as pseudo-code to demonstrate what this change would look like, as well as how easy it would be.

func (f *FlagSet) Var(value Value, names string, usage string) {
	// Remember the default value as a string; it won't change.
	for _, name := range strings.Split(names, ",") {
		name = strings.TrimSpace(name)
		
		flag := &Flag{name, usage, value, value.String()}
		_, alreadythere := f.formal[name]
		if alreadyThere {
			var msg string
			if f.name == "" {
				msg = fmt.Sprintf("flag redefined: %s", name)
			else {
				msg = fmt.Sprintf("%s flag redefined: %s", f.name, name)
			}
			fmt.Fprintln(f.Output(), msg)
			panic(msg) // Happens only if flags are declared with identical names
		}
		if f.formal == nil {
			f.formal = make(map[string]*Flag)
		}
		f.formal[name] = flag
	}
}

/*
	Example usage
	
	var exampleString string
	flag.StringVar(&exampleString, "test, t,testing", "", "This is an example")
	
	exampleString2 := flag.String("other_example,exampl", "", "This is another example")
*/

One potential issue I found with this proposal is that Go currently supports flags that are a whole word but have a comma in them (tested example). I couldn't find a way to produce this same result with any white-space characters, so I'm going to assume that a flag name with white-space is not possible. This would remove support for flag names that have commas in them, but that practice is far from conventional and the benefit of having multiple flag name support outweighs the harm that may do. It would also be backwards compatible (with the exception of flag names with commas) due to the fact strings.Split will just return the string if no comma is found. Another issue is that this would mean all the flag names that are defined in the same line would have the same description, but that is also a minor inconvenience compared to the benefits of having this functionality.

Applying this new method to the code found in /src/flag/example_test.go, it would look like this:

// Example 2: Two flags sharing a variable, so we can have a shorthand.
// The order of initialization is undefined, so make sure both use the
// same default value. They must be set up with an init function.
var gopherType string

func init() {
	const (
		defaultGopher = "pocket"
		usage         = "the variety of gopher"
	)
	flag.StringVar(&gopherType, "gopher_type, g", defaultGopher, usage)
}

I believe this is a much easier and more elegant way of completing the same task while sacrificing little to no compatibility
@gopherbot gopherbot added this to the Proposal milestone Nov 21, 2019
@turtletowerz turtletowerz changed the title proposal: flag: support comma-separated flag names defined in one line proposal: flag: support comma-separated flag aliasing Nov 21, 2019
@andybons
Copy link
Member

Can you please use actual code snippets and not images? Images are a far less accessible medium for representing code.

@turtletowerz
Copy link
Author

Can you please use actual code snippets and not images? Images are a far less accessible medium for representing code.

Switched from image to code block for the example

@mariush-github
Copy link

Would it make more sense to use | (bitwise or) instead of comma? It's just as unlikely to be used in parameter names.

@turtletowerz
Copy link
Author

Would it make more sense to use | (bitwise or) instead of comma? It's just as unlikely to be used in parameter names.

A vertical bar would probably be better than comma because you cannot create a single flag with it (ex. flag,flag2 is still considered one flag but flag|flag2 throws an error when the program is run)

If there is more support for the vertical bar being used as opposed to a comma I will update the original proposal with this idea. Thanks for the input!

@rsc
Copy link
Contributor

rsc commented Nov 27, 2019

Programs that want -c and --codec to mean the same thing usually also don't want --c or -codec to mean that thing. (Most often in those programs -codec means -c -o -d -e -c, assuming each of them is a boolean flag; or if -o takes an argument, it's -c -o=dec.)

If you want this kind of getopt/POSIX/GNU flag parsing, then there are plenty of packages that exist to do that. For example here's one that does that, using the standard flag package to manage definitions and flag registration: https://godoc.org/rsc.io/getopt.

If we added aliases like this to the standard library flag, it would make it look more like a getopt/etc flag parser, but it would still not be one (see handling of -codec). That seems like it would confuse more people than it would help.

Honestly, and this is a much more minor point, it also just seems like bad ergonomics. Users reading command lines need to know that -c and -codec are the same thing now, which doubles the API surface to memorize.

@rsc
Copy link
Contributor

rsc commented Nov 27, 2019

/cc @robpike

@rsc rsc closed this as completed Nov 27, 2019
@rsc rsc reopened this Nov 27, 2019
@rsc
Copy link
Contributor

rsc commented Nov 27, 2019

Sorry, accidentally clicked "Close and comment" instead of "Comment". Sheesh.

@rsc rsc added this to Incoming in Proposals (old) Nov 27, 2019
@rsc
Copy link
Contributor

rsc commented Nov 27, 2019 •
edited

Ian points out that you can also do:

s := flag.String("codec", "", "set codec to `name`")
flag.StringVar(s, "c", *s, "alias for -codec")

@rsc rsc mentioned this issue Nov 27, 2019
Open
@turtletowerz
Copy link
Author

If you want this kind of getopt/POSIX/GNU flag parsing, then there are plenty of packages that exist to do that. For example here's one that does that, using the standard flag package to manage definitions and flag registration: https://godoc.org/rsc.io/getopt.

I'm not a big fan of third-party dependencies which is why I proposed the change, but this is a relatively lightweight package that seems to do what I was looking for. Thanks!

@rsc rsc moved this from Incoming to Active in Proposals (old) Dec 4, 2019
@rsc rsc mentioned this issue Jul 15, 2020
Closed
@rsc rsc moved this from Active to Declined in Proposals (old) Sep 23, 2020
@golang golang locked and limited conversation to collaborators Nov 28, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge Proposal
Projects
No open projects
Proposals (old)
Declined
Milestone
Proposal
Development

No branches or pull requests
5 participants
@rsc @andybons @gopherbot @turtletowerz @mariush-github