-
Notifications
You must be signed in to change notification settings - Fork 98
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-completion: add completion script #261
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, a few comments. It's hard to review this without experience writing bash completion scripts, though.
I think I'm finally understanding most of this, but it's difficult to read because of the code duplication with the bash-completion boilerplate. Could you add some helper functions like:
... and use them everywhere where they can. Also, please use That should make things a lot easier to understand. E.g. the following:
... would become:
|
Also, please squash your commits into one. Thanks! |
I mainly:
Let me know if there is still something to do. |
It looks much better now, thanks. Can you address the two comments I just left (remove |
Thanks, merged now. |
Confer #260.