iot-lab / cli-tools

IoT-LAB CLI tools
Other
17 stars 14 forks source link

Add bash completion script #27

Closed audeoudh closed 6 years ago

aabadie commented 6 years ago

Nice addition, thanks ! I just tested it and it works like a charm.

For completeness, I think that you should rename the file to iotlab-cli-tools-bash-completion.sh, move it to an utils directory and add there a README.md explaining how to setup bash completion.

Finally, this PR breaks the CI, please exclude the script file from the license checker script.

aabadie commented 6 years ago

iotlab-profile and iotlab-robot are not managed by the script.

audeoudh commented 6 years ago

Yep, I set TODO at end of the completion file for these commands. It also does not support iotlab-ssh.

audeoudh commented 6 years ago

Should be OK for all iotlab-* tools.

There still remains some TODO and FIXME in file, for things not obvious or that need the IoT-lab API (as the API might be slow, it is not sure that completion is a good idea… however, I've not tested yet).

aabadie commented 6 years ago

iotlab-ssh support wasn't required here since this tool comes from another repository (ssh-cli-tools). Maybe you could also open a PR for iotlab-ssh there ?

aabadie commented 6 years ago

for setup notes, this is very system dependent. for example on debian like OS, the script file can simply be copied to the /etc/bash_completion.d directory.

audeoudh commented 6 years ago

Ok for iotlab-ssh. I did not notice this subtility. Do you think I need to separate the support of this command in its own file, to get rid of it in this repo? Or may I simply use the same file for both repositories?

audeoudh commented 6 years ago

For setup notes, I read in the readme of bash-completion that /etc/bash_completion.d is “only present for backwards compatibility”.

I just read again this part, and I note that I did not install the file correctly: they also say that “completions are loaded on-demand based on invoked commands' names, so be sure to name your completion file accordingly, and to include (for example) symbolic links in case the file provides completions for more than one command”. As the completion of iotlab-* commands uses the same helper functions, I prefer putting symbolic links to match all command names. I'll fix the readme and commit that soon.

aabadie commented 6 years ago

Do you think I need to separate the support of this command in its own file, to get rid of it in this repo? Or may I simply use the same file for both repositories?

For iotlab-ssh, a separate file should go to ssh-cli-tools. Even if it's just a copy paste of the iotlab-ssh related parts. In the future, we plan to merge to the 2 repositories in a single one. But for the moment, let's do this way:

Is it ok for you ?

audeoudh commented 6 years ago

Ok for me. I've updated my repo accordingly, and created a new PR in ssh-cli-tools.

aabadie commented 6 years ago

@audeoudh, I opened #30 to finalize your contribution. Initially I wanted to push directly in your branch but it seems that you forbid the access (or maybe it doesn't work on master branch).

I'm going to close this one.