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

Split long short options #1876

Closed
wants to merge 7 commits into from
Closed

Split long short options #1876

wants to merge 7 commits into from

Conversation

chriscool
Copy link
Contributor

This changes the option struct like this:

type option struct {
-       names       []string
+       longName    string
+       shortName   rune
        kind        reflect.Kind
        description string
 }

and make all the related changes.

This split was suggested in PR #1852 (Make ipfs commands --flags option).

A significant change is that the encoding global option has no more both the --enc and --encoding long names. Instead it has the -E short name and the --encoding long name.
If we really want the --enc long name too, we could perhaps do something like:

diff --git a/commands/option.go b/commands/option.go
index 3bb411a..e4a7b08 100644
--- a/commands/option.go
+++ b/commands/option.go
@@ -156,6 +156,7 @@ func (ov OptionValue) String() (value string, found bool, err error) {
 const (
        EncShort   = 'E'
        EncLong    = "encoding"
+       Enc2Long   = "enc"
        RecShort   = 'r'
        RecLong    = "recursive"
        ChanOpt    = "stream-channels"
@@ -164,6 +165,7 @@ const (

 // options that are used by this package
 var OptionEncodingType = StringOption(EncLong, EncShort, "The encoding type the output should be encoded with (json, xml, or text)")
+var OptionEncodingType2 = StringOption(Enc2Long, 0, "A synonym for " + EncLong)
 var OptionRecursivePath = BoolOption(RecLong, RecShort, "Add directory paths recursively")
 var OptionStreamChannels = BoolOption(ChanOpt, 0, "Stream channel output")
 var OptionTimeout = StringOption(TimeoutOpt, 0, "set a global timeout on the command")
@@ -171,6 +173,7 @@ var OptionTimeout = StringOption(TimeoutOpt, 0, "set a global timeout on the com
 // global options, added to every command
 var globalOptions = []Option{
        OptionEncodingType,
+       OptionEncodingType2,
        OptionStreamChannels,
        OptionTimeout,
 }

cc @whyrusleeping

License: MIT
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
License: MIT
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
License: MIT
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
License: MIT
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
License: MIT
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
License: MIT
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
License: MIT
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
@jbenet jbenet added the status/in-progress In progress label Oct 21, 2015
@whyrusleeping whyrusleeping added RFCR and removed status/in-progress In progress labels Oct 21, 2015
@cryptix
Copy link
Contributor

cryptix commented Oct 23, 2015

This LGTM without digging through the previous discussion. Mostly one-line changes, adding the new function argument.

@jbenet
Copy link
Member

jbenet commented Oct 27, 2015

I dont think this is a good idea. we may have more than two aliases, and the short code may be more than one char. there's lots of tools that run out of short codes that make clear sense, so they use two or three letters. or no short code makes sense. for example, something like

--access-control-allow-origin
--origin

or something.

@chriscool
Copy link
Contributor Author

Ok, let's close this then.

@chriscool chriscool closed this Oct 27, 2015
@chriscool chriscool deleted the split-long-short-options branch April 30, 2016 05:47
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

Successfully merging this pull request may close these issues.

None yet

4 participants