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

Bash completions for etcdctl #2892

Closed
wants to merge 3 commits into from
Closed

Conversation

eparis
Copy link
Contributor

@eparis eparis commented May 30, 2015

This does 2 things. First is ports from codegansta/cli to spf13/cobra.

Second it uses spf13/cobra to generate intelligent bash completions for etcdctl. The are generated by running

etcdctl genbash --outfile=yourfile and then sourcing the file . yourfile (or you could put the output in /etc/bash_completion.d/ or /usr/share/bash-completion/completions/ and have it automatically loaded.)

These completions are pretty cool because on my machine you can do obvious simple things:

# etcdctl [tab][tab]
backup          exec-watch      get             import          member          mkdir           rmdir           setdir          updatedir       
cluster-health  genbash         help            ls              mk              rm              set             update          watch

# etcdctl member [tab][tab]
add     list    remove  

But it also will show you data on 127.0.0.1!

# etcdctl ls /[tab][tab]
/coreos.com  /kube.local  /registry
# etcdctl ls /r[tab][tab]

completes

# etcdctl ls /registry/
/registry/controllers      /registry/minions          /registry/pods             /registry/serviceaccounts  
/registry/events           /registry/namespaces       /registry/ranges           /registry/services

It is NOT smart enough to use things you already types on your command line, so --peers etc aren't used trying to show the completions. Only loopback.

@eparis
Copy link
Contributor Author

eparis commented May 30, 2015

This is a WIP because I didn't immediately know how to handle --hidden to etcdctl import so I just disabled it. If people like this change, I'll figure that out. If not I thought, nah, forget it, yo, homes to Bel Air.

@eparis
Copy link
Contributor Author

eparis commented Jun 1, 2015

Also I realize I'll have to handle godeps 'the coreos' way. But I already opened another PR (#2893) to try to fix etcd's Godeps before I try to add more!

I could include that fix here, and give the complete solution, but review would be harder...

@eparis
Copy link
Contributor Author

eparis commented Jun 1, 2015

in case you want to try the results without fighting through building this code:

wget https://gist.githubusercontent.com/eparis/2380a2daa7ff6c0b52a6/raw/bdd185d78428b2fc9f97172066a9b4c29f9ff57d/gistfile1.sh
. gistfile1.sh
etcdctl [tab][tab]

@eparis
Copy link
Contributor Author

eparis commented Jun 1, 2015

updated, now with godeps, so building is easier/possible

//handleError(ErrorFromEtcd, err)
//}
//n += copyKeys(allh.Node, setc)
//}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

THIS MUST BE FIXED BEFORE MERGE

Copy link
Contributor

Choose a reason for hiding this comment

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

yea... another thing I noticed is that cobra does not support slice.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

https://github.com/eparis/pflag/tree/SliceStrings

Has support for string slices. I can define it in the etcdctl code, but that'd suck, so i'd consider this a prereq before I remove the WIP....

@xiang90
Copy link
Contributor

xiang90 commented Jun 1, 2015

@eparis I used cobra in my side project.

I found it is a little bit annoying (maybe it is configurable or my memory is wrong) is that I need to use "=" to separate the flag and the value if I use the long flag (--flag).

If I use short flag, it behaviors differently and unexpectedly. For example. I have a short flag "-d". If I use "-data", it will set -d flag to ata...

@eparis
Copy link
Contributor Author

eparis commented Jun 1, 2015

spf13/pflag#20 has a fix to the --flag "bob" not working.

I can probably poke steve to see what he thinks about me fixing it (add adding optional args at the same time)

-data is 'expected' if -d isn't a boolean... i don't see a way 'fix' that....

@xiang90
Copy link
Contributor

xiang90 commented Jun 1, 2015

@eparis My argument is short and long behavior differently...

Look this, I have a flag (data, d). The app simply prints out the flag value.

./myapp --dataddd
Error: unknown flag: --dataddd
Run 'myapp help' for usage.
./myapp --data ddd
Error: flag needs an argument: --data
Run 'myapp help' for usage.
./myapp -dataddd
ataddd
./myapp -d dd
dd

@eparis
Copy link
Contributor Author

eparis commented Jun 1, 2015

@xiang90 that's called posix flag parsing. I get it, but that's the way unix does it. Which is why I don't think it can be 'fixed'

# 11 entries in dir
$ ls | wc -l
11

# ignore one of them
$ ls --ignore bob | wc -l
10

# can't just mash them together with long args
$ ls --ignorebob | wc -l
ls: unrecognized option '--ignorebob'
Try 'ls --help' for more information.
0

# space wtih short args works
$ ls -I bob | wc -l
10

# and with short args you can just mash them together
$ ls -Ibob | wc -l
10

@xiang90
Copy link
Contributor

xiang90 commented Jun 1, 2015

@eparis OK. So I will be happy to take a further look at this when:

  1. slice flag is resolved
  2. --foo "Bob" works.

Sounds good?

@eparis
Copy link
Contributor Author

eparis commented Jun 1, 2015

did you play with the completion? was it useful enough for me to keep pushing? (at the least --foo "bob" needs fixed no matter what!)

@xiang90
Copy link
Contributor

xiang90 commented Jun 1, 2015

@eparis Yea. I play with it. I think it is quite useful. But to be honest, I use etcd client more often than ctl as a developer. (So I can never remember those commands and flags although I wrote some of them)

@philips Probably you have an opinion on the completions?

@eparis eparis force-pushed the bash-completions branch 2 times, most recently from 0eecc95 to a276035 Compare June 30, 2015 00:19
@eparis
Copy link
Contributor Author

eparis commented Jun 30, 2015

All comments should be addressed and this should be g2g....

@eparis eparis changed the title WIP: DO NOT MERGE: Bash completions for etcdctl Bash completions for etcdctl Jun 30, 2015
@xiang90
Copy link
Contributor

xiang90 commented Jun 30, 2015

@eparis This is too late for 2.1. Can you send a pull request to dev 2.2 branch?

@gyuho
Copy link
Contributor

gyuho commented Jan 13, 2016

@eparis Any updates on this?

BTW, we have migrated to spf13/cobra for V3.

Please check that out if you have time.

@xiang90
Copy link
Contributor

xiang90 commented Jun 22, 2016

Closing this since it is inactive.

@xiang90 xiang90 closed this Jun 22, 2016
@tpo
Copy link

tpo commented Jun 9, 2020

@eparis a pitty this hasn't gone in. I'd love to have etcdctl bash completion. Any chance you pick this up again and try to push this into etcd?

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

Successfully merging this pull request may close these issues.

5 participants