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

Generate shell completions and man page #122

Merged
merged 5 commits into from
Oct 31, 2021
Merged

Generate shell completions and man page #122

merged 5 commits into from
Oct 31, 2021

Conversation

figsoda
Copy link
Member

@figsoda figsoda commented Oct 20, 2021

Moved some parts of cli.rs to opts.rs, ouch::cli::{Opts, QuestionPolicy, Subcommand} are moved to ouch::{Opts, QuestionPolicy, Subcommand}

This uses a build script to generate completions when GEN_COMPLETIONS=1 is present, this generates completions to $OUT_DIR/completions, for example:

  • GEN_COMPLETIONS=1 cargo build would generate completions to target/debug/build/ouch-*/out/completions
  • GEN_COMPLETIONS=1 cargo build --release --target x86_64-unknown-linux-musl would generate completions to target/x86_64-unknown-linux-musl/release/build/ouch-*/out/completions

A few more artifacts are generated in the completions in the x86_64-musl job:

  • completions are generated and uploaded to completions
  • man pages are generated with help2man and uploaded to ouch.1

Design choices:

  • I chose completions to be generated to OUT_DIR, which is what rust/cargo recommends, but it also makes paths incredibly long and can possibly collide (when globbing with *) when completions are generated multiple times
  • Man page could also be gzipped
  • Completions could be in multiple artifacts, and powershell and zsh completions are named slightly intuitively since I left them as defaults

Side notes:

  • I couldn't figure out a way for man pages to be generated inside build.rs since help2man excepts an executable, not a string
  • The current ci is very big and repetitive, I have a very similar workflow that tests, builds, and creates releases on multiple platforms that we can probably borrow from: https://github.com/figsoda/mmtc/tree/main/.github/workflows

@figsoda
Copy link
Member Author

figsoda commented Oct 21, 2021

resolved the merge conflicts

@marcospb19
Copy link
Member

Nice PR!

The current ci is very big and repetitive, I have a very similar workflow that tests, builds, and creates releases on multiple platforms that we can probably borrow from: https://github.com/figsoda/mmtc/tree/main/.github/workflows

Just saw your CI file and learned some stuff from it, seems like you can upload artifacts for each different job while keeping the different name for each of them, but can you optionally not upload the binaries for some of the jobs? (like we do currently).

I chose completions to be generated to OUT_DIR, which is what rust/cargo recommends, but it also makes paths incredibly long and can possibly collide (when globbing with *) when completions are generated multiple times

The odds of this colliding is high?

Man page could also be gzipped

Would be nice if you could gzip them CI before uploading, but I'm a noob when it comes to CI, is it easy to do? (@vrmiguel managed to learn github actions and did everything for ouch, I did practically nothing).

@figsoda
Copy link
Member Author

figsoda commented Oct 21, 2021

seems like you can upload artifacts for each different job while keeping the different name for each of them

Actually I don't upload any artifacts, the jobs only upload stuff to the releases

but can you optionally not upload the binaries for some of the jobs?

Should be possible, the main difference is more uses of matrix

The odds of this colliding is high?

If you run cargo build with GEN_COMPLETIONS multiple times, trying to find the completion directory with *s will find you multiple directories, and there is no easy way to find out which is the newest one. This shouldn't be a issue in ci, but might be a little annoying when testing changes to completions

Would be nice if you could gzip them CI before uploading

I should've mentioned that artifacts are in .zip formats, should I still gzip it into ouch.1.gz and have it be stored in a zip? And yes it is easy to do

@figsoda figsoda mentioned this pull request Oct 22, 2021
@marcospb19
Copy link
Member

I should've mentioned that artifacts are in .zip formats, should I still gzip it into ouch.1.gz and have it be stored in a zip?

Oh yes, there's this, so no, leave them uncompressed, I'll probably need to create a script to help with releases anyway, so I could add this step to it.

build.rs Show resolved Hide resolved
@figsoda figsoda requested a review from marcospb19 October 22, 2021 14:44
@figsoda
Copy link
Member Author

figsoda commented Oct 30, 2021

resolved merge conflicts

@marcospb19 marcospb19 merged commit a85eb68 into ouch-org:master Oct 31, 2021
@marcospb19
Copy link
Member

Resolved some more conflicts that appeared, thanks for the contribution and sorry for the delay!

@marcospb19 marcospb19 added the hacktoberfest-accepted Tag PR as accepted for the hacktoberfest event label Oct 31, 2021
@marcospb19 marcospb19 changed the title Generate completions and man page Generate shell completions and man page Oct 31, 2021
@marcospb19
Copy link
Member

@figsoda have you tried out the zsh completion? Bash is working here but zsh is not.

@figsoda
Copy link
Member Author

figsoda commented Nov 2, 2021

I did not, I don't use zsh

Fish worked fine for me

@figsoda figsoda deleted the completions-manpage branch November 2, 2021 21:45
@yigitsever
Copy link

zsh completion is not working because of the following in _ouch (line 40 on my install);

':output -- The resulting file. It's extensions can be used to specify the compression formats:_files' \

The ' in It's is causing the problem, replacing the line as;

":output -- The resulting file. It's extensions can be used to specify the compression formats:_files" \

Fixes the issue, I don't know about the generation process so just fixed it by hand in the meantime.

@figsoda
Copy link
Member Author

figsoda commented Nov 3, 2021

@yigitsever does this fix it for you?
#153

@yigitsever
Copy link

@figsoda it does, didn't even realize the typo...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
hacktoberfest-accepted Tag PR as accepted for the hacktoberfest event
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants