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 completion #372

Closed
wants to merge 1 commit into from
Closed

Conversation

davidszotten
Copy link
Contributor

clap can now generate bash completion. not sure what the best way to ship this is, but at least we can generate them

$ diesel<TAB>
--database-url  --version       -h              help            setup
--help          -V              database        migration

$ diesel migration generate<TAB>
--help            --version         -h                <MIGRATION_NAME>

@sgrif
Copy link
Member

sgrif commented Jul 4, 2016

/cc @mcasper I think we should give a command to generate at runtime since users won't have access to the files in build.rs

@davidszotten
Copy link
Contributor Author

i'm pretty new to rust. is there no way for e.g. cargo install to ship the completion script as well?

@killercup
Copy link
Member

killercup commented Jul 4, 2016

AFAIK cargo install only installs the binary. There is rust-lang/cargo#2729 open requesting to install other files.

While I would like to have this, I'd probably just (a) want to use a system package manager (brew, apt; long term thing) or (b) install diesel-cli myself from the repository (short term and while developing diesel itself of course).

@davidszotten
Copy link
Contributor Author

as a user of diesel (and by definition a rust developer), cargo install seems like a perfectly fine way to install the cli to me. it's unfortunate that we can't ship the generated completion script, and, it seems the only entry point to gen_completions is to give it an output dir. this seems unintuitive to me (e.g. "please run diesel generate-bash-completion /etc/bash_completion.d to have it write a file in that directory)

thoughts on such an api? might it be possible for the build.rs in this pr to read the generated bash code and then compile the contents into the app (to support something like diesel generate-bash-completion > /etc/bash_completion.d/diesel

Bash completion
---------------

Diesel can generate a bash completion script for the itself:
Copy link
Member

Choose a reason for hiding this comment

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

probably: s/for the itself/for the CLI

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yup. thanks

@davidszotten
Copy link
Contributor Author

don't love generate-bash-completion as a name so open to suggestions. also, should it be "hidden" under some sub-command?

@davidszotten
Copy link
Contributor Author

looks like rustup failed on one of the travis jobs. think you need more access than i have to re-trigger the build

@davidszotten
Copy link
Contributor Author

clap can now generate completions to a string; no need for codegen

@davidszotten
Copy link
Contributor Author

just to follow up on this. if this feature isn't suitable for this project, please let me know and i'll close the pr. if this is interesting, but there are issues with the implementation, i'll happily work with you to fix/improve

@mcasper
Copy link
Contributor

mcasper commented Aug 29, 2016

Sorry for the long absence, I think I agree that this is the best approach we can support within Diesel right now.

Maybe shorten the command name to just bash-completion? To me it just seems a little friendlier to type, and most users should be familiar with what it is.

Also, looks like there are some conflicts to deal with now :/

@davidszotten
Copy link
Contributor Author

no worries. command shortened and commits rebased+squashed

sgrif added a commit that referenced this pull request Aug 29, 2016
@sgrif
Copy link
Member

sgrif commented Aug 29, 2016

Thanks. I manually merged this in 17326dd. There were some style issues that needed to be addressed which I did by amending your commit to avoid git churn. As such Github won't automatically pick up on me merging it, but it has been accepted. Thank you for working on this.

@sgrif sgrif closed this Aug 29, 2016
@davidszotten
Copy link
Contributor Author

thanks!

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

Successfully merging this pull request may close these issues.

4 participants