Add support for specifying only a shortflag by cornfeedhobo · Pull Request #171 · spf13/pflag

cornfeedhobo · 2018-06-24T04:14:16Z

This PR seeks to add support for specifying flags with only a short flag. Tests have been adjusted and added, but suggestions for any additional validation are welcome.

Fixes #139
Closes #165
Fixes spf13/cobra#679

CLAassistant · 2018-06-24T04:14:23Z

All committers have signed the CLA.

cornfeedhobo · 2018-06-27T01:07:49Z

@spf13 @eparis This is ready for review.

cornfeedhobo · 2018-10-19T17:55:20Z

@spf13 @eparis This is ready for review. Any chance you could take a look?

TerraTech · 2019-04-28T15:35:59Z

@spf13 @eparis Any chance of this patch will be merged soon?

Nuru · 2019-10-03T22:26:31Z

I'm happy to have the option to have short flags without requiring long synonyms, but I would like boolean short flags to be handled more like non-boolean short flags for consistency. The current state where a flag with an argument is written -f file but a boolean cannot be written -b false but instead has to be -b=false is painfully inconsistent as well as awkward.

There are 2 options that make sense to me and it would be reasonable to implement either or both:

Option 1, allow the value to be specified as the next word, without an equals sign, just as with -f file. If the word following a boolean option is "true" or "false", take that as the value for the option, otherwise do not associate that word with the option. So in a hypothetical echo command where -b makes the output bold:

$ echo -b bold
bold # that is "bold" output in bold
$ echo -b false bold
bold # that is "bold" output in plain type

Option 2, allow negating a short option with a long option, so that boolean options can be set either way without an argument. Create a (possibly hidden) inverse long option using the no prefix.

$ echo -b bold
bold # that is "bold" output in bold
$ echo --no-b bold
bold # that is "bold" output in plain type

The reason it is important to have a way of explicitly setting an option to its default value (e.g. --no-b) is because many commands take configuration settings from many places, such as config files and environment variables, and so you want to have a command line option to override such settings and restore the default.

BTW, this looks like this would also address spf13/cobra#918

cornfeedhobo · 2019-11-05T12:05:37Z

@Nuru is this concern specific to my branch?

Edit: I haven't looked at this in some time. I see what you mean. I'll think about it.

Nuru · 2019-11-08T22:03:05Z

@Nuru is this concern specific to my branch?

@cornfeedhobo It is not entirely specific to your branch, but without a long name for the flag, this syntax I am complaining about is the only way left to override an option that defaults to true, so I would say that without meaning to, your branch/PR makes the situation worse.

cornfeedhobo · 2019-11-10T17:57:59Z

@Nuru After some experimentation, I've decided that your concern is outside the scope of this PR. If someone is passing values, they already must know this is a boolean flag, so they can easily add the = delimiter.

Edit: I'll take a look at #214 and see if I can implement that.

Nuru · 2019-11-16T00:23:00Z

Thinking more about it, I'm fully opposed to allowing short flags only. The only benefit to not having long options is slightly shorter help text. I think allowing short arguments without long arguments is problematic for a general utility such as this that goes into tools like cobra and viper which support multiple levels of configuration (config file, environment variables, command line), because the semantics (or at least the customary usage) of short flags is command-line only. Users wanting to override on the command line values specified elsewhere need to be able to set options back to defaults, and that is only customarily possible with long options.

I'm tempted to write a new parsing library to implement options the way I think they should be, but I have other priorities at the moment. Meanwhile, you can see my suggestion for how to handle spf13/cobra#679 (comment) and #139 (comment)

cornfeedhobo · 2019-11-16T03:43:02Z

Haha, sounds like we'll have to agree to disagree. I will simply say that I think you are looking at this PR through your own narrow viewpoint, and that there are many valid reasons to support just short flags, especially since it's just another option, no in any way forced upon the user.

Nuru · 2019-11-16T04:06:21Z

@cornfeedhobo I'd love to hear some of your "many valid reasons" to deny users the option of using a long flag and associated long flag syntax. From my point of view, if you have a short option -D there is no reason not to automatically create a long option --D if you cannot come up with a different name. it would be super confusing for -D and --D to be 2 different options, so what do you see as the down side?

If you require short flags to also have long flags, then you can solve a lot of issues with consistency of syntax and ambiguity of parsing by limiting certain things to long flags. For example a boolean short flag -D can then be only -D to set it true, but only --D=false or --no-Dto set it false. We avoid having to wonder if -Dfalse is the same as -Dalsef or not. And several other problematic use cases also go away.

cornfeedhobo · 2019-11-18T02:29:54Z

@Nuru

I'd love to hear some of your "many valid reasons" to deny users the option of using a long flag and associated long flag syntax.

No one is "deny"ing anything. This makes the flag library offer the same capability as the built-in flag library.

Unfortunately, you're way too passionate to engage in this discussion further. It's out of the scope of my PR and it looks like you just want someone to argue with. Furthermore, this PR has been open for over a year and has no hope of being merged in sight and even if it is merged it has literally no affect on you.

cornfeedhobo · 2020-02-18T18:23:32Z

@therealmitchconnors any way to get some attention here?

jharshman · 2020-02-26T17:03:58Z

@spf13 looks like this issue has been open for a while.
The OP is looking for a response.

xxxserxxx · 2020-04-27T17:41:30Z

@cornfeedhobo Any chance you are interested in maintaining a fork with this patch in it?

cornfeedhobo · 2020-04-28T22:05:30Z

@xxxserxxx Yeah I'm considering it. I tried to discuss the issue over at spf13/cobra#1050, but as you can see, @jharshman completely shut down the conversation, and I'm kinda scared to push the issue now. This is the first time I've had to deal with censorship in opensource. I encourage you to push him on the matter.

jharshman · 2020-04-28T22:25:22Z

@cornfeedhobo

@jharshman completely shut down the conversation, and I'm kinda scared to push the issue now.

Yes that was the incorrect place to raise the issue.

This is the first time I've had to deal with censorship in opensource. I encourage you to push him on the matter.

I don't think you understand what censorship is. Since your comment on spf13/cobra#1050, I have obtained collaborator access to this repository with the intent of triaging open issues and Pull Requests. However, these things take time and other items have taken priority.

Thank you for your patience.

cornfeedhobo · 2020-04-28T22:33:54Z

@jharshman Um, you chose to lock down an issue from comments when that isn't normal practice. That is literally censorship.

Yes that was the incorrect place to raise the issue.

Cobra is tightly coupled to pflag - there is no interface allowing a drop in replacement for it. I don't know where else would have been a proper place to discuss forking and moving.

However, these things take time and other items have taken priority.
Thank you for your patience.

I think 2 years is extremely patient, and as you can see, I am not the first to discuss forking this project. In fact, most people I have discussed this with say that's what they do at their company. Alternatively, Cobra could remove this tight coupling and we wouldn't be having this issue. Either way, you've locked users into a rock and a hard place. I think my actions have been extremely polite considering the length of time, neglect, and importance of this library.

jharshman

I don't think the implementation is quite there yet.
I'd rather this take the form of a new method(s) on FlagSet explicitly intended to use only short flags.

jharshman · 2020-04-28T22:46:05Z

 		if flag.Shorthand != "" && flag.ShorthandDeprecated == "" {
-			flagName = fmt.Sprintf("-%s, --%s", flag.Shorthand, flag.Name)
+			if flag.shorthandOnly {
+				flagName = fmt.Sprintf("-%s", flag.Shorthand)


This it technically just suppressing the long name for the flag.
Is there a setup where the flag only gets a shorthand? Something where you wouldn't have to explicitly specify "" for the flag name?

Okay, I've started another branch that will take a more explicit approach. I'll post it soon, but it's a much larger diff.

cornfeedhobo · 2020-05-07T01:55:44Z

@jharshman Please take a look at #256 and see if that is more suitable.

This commit adds S suffixed methods to support the creation of flags that are only exposed to the user in their shorthand form. Fixes spf13#256 Closes spf13#171

This commit adds S suffixed methods to support the creation of flags that are only exposed to the user in their shorthand form. This also includes a full standardization of all the wrappers. Fixes spf13#256 Closes spf13#171

m4saurabh · 2025-02-11T15:30:18Z

Just use https://github.com/urfave/cli/ if you are starting out because this feature is not supported

Add support for specifying only a shortflag

a243da4

cornfeedhobo added 2 commits June 24, 2018 00:02

update print usage test for shortonly support

96f8921

add documentation of shortflag only support

43097a3

umarcor mentioned this pull request Sep 11, 2019

Please allow booleans to be set false with no- prefix spf13/cobra#958

Closed

This was referenced Oct 3, 2019

any possible to have only argument's shorthand #213

Closed

Allow short flags without long form #139

Open

Nuru mentioned this pull request Jan 10, 2020

Support for short flag only? spf13/cobra#679

Closed

cornfeedhobo requested a review from eparis February 19, 2020 02:37

cornfeedhobo mentioned this pull request Feb 25, 2020

Support for short flags only spf13/cobra#1043

Closed

jharshman suggested changes Apr 28, 2020

View reviewed changes

jharshman reviewed Apr 28, 2020

View reviewed changes

Merge branch 'master' into add-shortflag-only-support

2740611

cornfeedhobo mentioned this pull request Aug 24, 2020

Add support for specifying only a shortflag #256

Closed

cornfeedhobo closed this Sep 20, 2020

cornfeedhobo deleted the add-shortflag-only-support branch September 20, 2020 12:24

Conversation

cornfeedhobo commented Jun 24, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

CLAassistant commented Jun 24, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cornfeedhobo commented Jun 27, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cornfeedhobo commented Oct 19, 2018

Uh oh!

TerraTech commented Apr 28, 2019

Uh oh!

Nuru commented Oct 3, 2019

Uh oh!

cornfeedhobo commented Nov 5, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Nuru commented Nov 8, 2019

Uh oh!

cornfeedhobo commented Nov 10, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Nuru commented Nov 16, 2019

Uh oh!

cornfeedhobo commented Nov 16, 2019

Uh oh!

Nuru commented Nov 16, 2019

Uh oh!

cornfeedhobo commented Nov 18, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cornfeedhobo commented Feb 18, 2020

Uh oh!

jharshman commented Feb 26, 2020

Uh oh!

xxxserxxx commented Apr 27, 2020

Uh oh!

cornfeedhobo commented Apr 28, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jharshman commented Apr 28, 2020

Uh oh!

cornfeedhobo commented Apr 28, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jharshman left a comment

Choose a reason for hiding this comment

Uh oh!

jharshman Apr 28, 2020

Choose a reason for hiding this comment

Uh oh!

cornfeedhobo May 1, 2020

Choose a reason for hiding this comment

Uh oh!

cornfeedhobo commented May 7, 2020

Uh oh!

m4saurabh commented Feb 11, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

cornfeedhobo commented Jun 24, 2018 •

edited

Loading

CLAassistant commented Jun 24, 2018 •

edited

Loading

cornfeedhobo commented Jun 27, 2018 •

edited

Loading

cornfeedhobo commented Nov 5, 2019 •

edited

Loading

cornfeedhobo commented Nov 10, 2019 •

edited

Loading

cornfeedhobo commented Nov 18, 2019 •

edited

Loading

cornfeedhobo commented Apr 28, 2020 •

edited

Loading

cornfeedhobo commented Apr 28, 2020 •

edited

Loading