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

Add flag category support #796

Closed

Conversation

michaeljs1990
Copy link
Contributor

@michaeljs1990 michaeljs1990 commented Jan 27, 2019

This could be better and needs some cleanup as well as tests. It gives you the basic ability that I wanted which lets you set a category on a flag and have it printed in the help to break up commands with super long help messages. Global options ignore this right now but could be made to use it if someone messed with the template.

Here is an example output with this patch applied.

NAME:
   collins-go-cli query - Search for assets in Collins

USAGE:
   collins-go-cli query [command options] [arguments...]

OPTIONS:
  
   -Z, --remote-lookup                Query remote datacenters for asset
   -T value, --type value             Only show asset with type value
   -n value, --nodeclass value        Assets in nodeclass value[,...]
   -p value, --pool value             Assets in pool value[,...]
   -s value, --size value             Number of assets to return per page (default: 100)
   --limit value                      Limit total results of assets (default: 0)
   -r value, --role value             Assets in primary role
   -R value, --secondary-role value   Assets in secondary role
   -i value, --ip-address value       Assets with IP address[es]
   -S value, --status value           Asset status (and optional state after :)
   -a value, --attribute value        Arbitrary attributes and values to match in query. : between key and value
   -H, --show-header                  Show header fields in output
   -c value, --columns value          Attributes to output as columns, comma separated (default: "tag,hostname,nodeclass,status,pool,primary_role,secondary_role")
   -x value, --extra-columns value    Show these columns in addition to the default columns, comma separated
   -f value, --field-separator value  Separator between columns in output
   -l, --link                         Output link to assets found in web UI
   -j, --json                         Output results in JSON
   -y, --yaml                         Output results in YAML
   
  Test
   -t value, --tag value  Assets with tag[s] value[,...]
   

It also adds go mod support so I can overwrite the package locally with the mod replace syntax.

This adds what I think needs to be done to support categories for flags
but we will see if that works. It also forces the scripts to use python2
since they blow up under python3 which is becoming the default python on
many linux systems.

Small fix to app_test as well so it conforms to the new Flag interface.
Let us use this repo via replace locally.
@michaeljs1990
Copy link
Contributor Author

michaeljs1990 commented Jan 27, 2019

Additionally I made some changes to the python generator scripts since python3 is becoming fairly common as the default python. The script currently blows up under python3 though.

I also have some cleanup to do around methods I added that weren't actually needed after digging deeper into the code base.

@michaeljs1990 michaeljs1990 mentioned this pull request Jan 27, 2019
@michaeljs1990
Copy link
Contributor Author

michaeljs1990 commented Jan 27, 2019

If you want to use this before it lands since review seems a bit slow on this project right now you can use the following which is an example from my go project. go1.11 required for it.

module cgit.xrt0x.com/xrt0x/collins-go-cli

replace github.com/urfave/cli => github.com/michaeljs1990/cli v1.20.1-0.20190127222559-50b52ca9d52b

require (
        github.com/google/go-querystring v1.0.0 // indirect
        github.com/schallert/iso8601 v0.0.0-20151102174922-d51701471974 // indirect
        github.com/sirupsen/logrus v1.3.0
        github.com/urfave/cli v1.20.0
        gopkg.in/tumblr/go-collins.v0 v0.0.0-20180412191224-96f4cd6792f7
)

@michaeljs1990
Copy link
Contributor Author

michaeljs1990 commented Jan 28, 2019

I was looking at adding global flag support for sub commands but it breaks a lot of unit tests so I'll move it into another PR.

@michaeljs1990 michaeljs1990 force-pushed the add-flag-category-support branch from b0494d5 to 50b52ca Compare Jan 28, 2019
@endersonmaia
Copy link

endersonmaia commented Mar 21, 2019

I'd split the go.mod and go.sum to another PR

@michaeljs1990
Copy link
Contributor Author

michaeljs1990 commented Mar 21, 2019

I'd split the go.mod and go.sum to another PR

Ah yeah, that just leaked in since I was using it locally for making testing it in my project easier. I'll clean that up.

fc = fc.AddFlag(flag.GetCategory(), flag)
}

sort.Sort(fc)
Copy link

@akraxx akraxx Apr 1, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello,

It is possible to add an option to be able to disable default sorting? I would like to sort categories by appearance :

...
OPTIONS:
  scope
   --scope1 value     object_code
   --scope2 value   object test

  mandatory
   --john value  JJ
   --doe value       DD
Flags: []cli.Flag{
			cli.StringFlag{
				Name:  "scope1",
				Category: "scope",
			},
			cli.StringFlag{
				Name:  "scope2",
				Category: "scope",
			},
			cli.StringFlag{
				Name:  "john",
				Category: "mandatory",
			},
			cli.StringFlag{
				Name:  "doe",
				Category: "mandatory",
			},

Thanks !

@coilysiren
Copy link
Member

coilysiren commented Aug 17, 2019

Heya! Looks like this PR has gotten fairly outdated. If you're still interested, can you rebase it?

Also for the sake of readers, add a "Closes #259" to the top of the description?

@coilysiren
Copy link
Member

coilysiren commented Aug 24, 2019

Closing pending user feedback!

@coilysiren coilysiren closed this Aug 24, 2019
@AkihiroSuda
Copy link
Contributor

AkihiroSuda commented Feb 26, 2021

Any chance to get this reopened?

@michaeljs1990
Copy link
Contributor Author

michaeljs1990 commented Mar 1, 2021

Any chance to get this reopened?

Sorry I had a fork that I was using and am still using for my project that adds this and I didn't need any more work done. Feel free to take this though and open another PR.

@meatballhat
Copy link
Member

meatballhat commented Apr 22, 2022

@michaeljs1990 Sorry for the very long delay 😭 I'm picking this up and attempting to get your changes incorporated into the current main / v2 line.

@meatballhat meatballhat self-assigned this Apr 22, 2022
@meatballhat meatballhat added this to the Release 2.5.0 milestone Apr 22, 2022
@meatballhat meatballhat added kind/feature describes a code enhancement / feature request area/v2 relates to / is being considered for v2 labels Apr 22, 2022
meatballhat added a commit that referenced this pull request Apr 22, 2022
@meatballhat meatballhat removed this from the Release 2.5.0 milestone Apr 26, 2022
@meatballhat meatballhat added the status/superseded has been superseded label May 7, 2022
meatballhat added a commit that referenced this pull request May 22, 2022
@meatballhat
Copy link
Member

meatballhat commented May 22, 2022

Merged via #1368

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/feature describes a code enhancement / feature request status/superseded has been superseded
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants