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

[enh] bash completion for yunohost cli #678

Merged
merged 4 commits into from
Apr 25, 2019
Merged

[enh] bash completion for yunohost cli #678

merged 4 commits into from
Apr 25, 2019

Conversation

ChristopheVuillot
Copy link
Contributor

@ChristopheVuillot ChristopheVuillot commented Mar 11, 2019

Added a python script (yunohost_completion.py) which generates a bash completion file for the yunohost command based on yunohost.yml, in data/actionsmap

Added the output of the script in data/bash-completion.d/yunohost_completion

This is probably not the correct place for the script and the generation should
be done at some other time and place also.

The problem

missing bash completion for yunohost command

Solution

a python script which generates the file from yunohost.yml

PR Status

[WIP]

How to test

Source the generated file:
source yunohost_completion
then you should be able to auto complete

To regenerate the file:
python yunohost_completion.py

Validation

  • Principle agreement 0/2 :
  • Quick review 0/1 :
  • Simple test 0/1 :
  • Deep review 0/1 :

… completion file for the yunohost command based on yunohost.yml, in data/actionsmap

Added the output of the script in data/bash-completion.d/yunohost_completion

This is probably not the correct place for the script and the generation should
be done at some other time and place also.
@Psycojoker
Copy link
Member

Again, thx a lot for that work :)

@ChristopheVuillot
Copy link
Contributor Author

Very happy to contribute :).

Thanks for adding the labels, it indeed needs reviews and tests. I don't know where the script should be put and how it should be run.

For tests, I guess, at least checking that the generated file is syntactically correct could be a start ?

@alexAubin
Copy link
Member

alexAubin commented Mar 25, 2019

Hey there ! Finally taking the time to have a first look at this ! Thank so much for this, I think many people will find it pleasing to have auto completion !

I'm thinking maybe we can add something to be ran during debian's build to automatically generate the file ?

Also since you flagged this as "wip" is there anything else you were thinking about implementing ?

I'm gonna test this during the coming days

@ChristopheVuillot
Copy link
Contributor Author

Hey, thanks for having a look.
For the [wip] flag I wasn't sure if I needed to put it, I just put it there because this cannot be merged as is, but as you say something needs to be added to run the script. Although I don't know how debian's build works so would need some help on that.
As far as the script is concerned I don't intend to add anything else for now.

@alexAubin
Copy link
Member

Alrighty 😉

Indeed the whole Debian build thing ain't trivial at all ! I will have a look soon™

@alexAubin
Copy link
Member

Did a few tweaks, still need to test that running the script during debian builds does work

@alexAubin alexAubin added this to the 3.6.x milestone Apr 1, 2019
@ChristopheVuillot
Copy link
Contributor Author

Just fixed some names for coherence.

@ChristopheVuillot ChristopheVuillot changed the title [WIP] [enh] bash completion for yunohost cli [enh] bash completion for yunohost cli Apr 12, 2019
@alexAubin
Copy link
Member

Sooooo tested the debian build and it seems to work okay 👌

That looks good for me, planning to merge soon

Note though that not all setups load the bash-completion thing (I believe some commands need to be ran inside the .bashrc of root/admin or something like this ...). There's no trivial solution for this tho ... But we could imagine something to add those command during the initial install.

@ChristopheVuillot
Copy link
Contributor Author

Thank you! I think that all the bash-completions are in principle loaded at boot, but otherwise one just has to source the thing once. So maybe putting it in .bashrc is a bit too much but just source the completion file during the initial install may be better ?

@alexAubin
Copy link
Member

Uuuh I doubt that bash completion is loaded "globally" ? To me it's something that must be sourced by the shell one way or another ...

On my machine, I see there's a /etc/profile.d/bash_completion.sh which is sourced by /etc/profile, which seems to be something like a default thing loaded by all shell when they start ... But my point was mainly that probably not all setups have this enabled by default ... Maybe it's just a matter of installing a bash-completion package, idk :s

@ChristopheVuillot
Copy link
Contributor Author

Oh yes you are right, sorry, I got confused. I don't know what is the default for debian actually. Probably people that want to use completion in general have already enabled it.

@alexAubin alexAubin merged commit 3b5b7a9 into YunoHost:stretch-unstable Apr 25, 2019
@alexAubin
Copy link
Member

alexAubin commented Apr 25, 2019

So in fact I noticed that there is indeed a bash-completion package for debian which actually is in recommended packages for yunohost :

> aptitude why bash-completion
i   yunohost Recommends bash-completion

So modulo the "funny" setups who do not install recommended packages (that should be solved, c.f. discussion on YunoHost/issues#1320 ... ) the bash completion should be enabled on all setups I think o/

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

Successfully merging this pull request may close these issues.

3 participants