-
Couldn't load subscription status.
- Fork 399
VBoxManage completion #178
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
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.
Here's a very quick peek. In addition to inline comments, Makefile.am changes are missing, as are the test cases.
Additionally, this is such a huge completion I don't use myself, so the standard questions apply: have you considered submitting this for inclusion in upstream? If not, would you be willing to actively maintain this in bash-completion in the future, i.e. have your commit access re-enabled?
completions/VBoxManage
Outdated
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.
Should be Should be _vboxmanage_get_ostype (no trailing "s").
completions/VBoxManage
Outdated
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.
Please replace these with plain returns (no 0) or consistency with rest of the tree.
completions/VBoxManage
Outdated
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.
Can't these huge hardcoded lists be parsed with e.g. _parse_help?
Oh, and the commit message needs work, something like "VBoxManage: New completion" would be appropriate.
e2a74f5 to
efad254
Compare
Sorry for the answer delay. Here is a new proposal, addressing most (if not all) comments.
And I'm volonteering to maintain it if needed, of course.
gryf
commented
Oct 8, 2018
There is at least several different bash completions for vboxmanage with different state and quality. I'm wondering if there is a point to keep such complicated completion. Bash-completion itself have separate package at most Linux systems, so the VirtualBox. In perfect world vboxmanage completion should match version of installed VirtualBox, since completion may offer options which doesn't exists in in installed version of VirtualBox, or on the other hand, it may not provide right completion, if completion is too old.
I see the point to have it, but I'd rather have the right version matching installed VirtualBox, than a broken one.
Also, depending o Linux distribution, it can happen, that they main line will contain different version of VirtualBox.
Disclaimer: I'm the author of another vboxmanage completion script.
c0e9459 to
09307f8
Compare
No description provided.