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

Add shell completions #2107

Merged

Conversation

hasufell
Copy link
Contributor

@hasufell hasufell commented Sep 2, 2020

@hasufell hasufell requested a review from KtorZ September 2, 2020 19:11
@KtorZ
Copy link
Member

KtorZ commented Sep 3, 2020

This is already mentioned in our user guide: https://github.com/input-output-hk/cardano-wallet/wiki/Wallet-command-line-interface

Also, I'd rather give the command to generate the auto-completion scripts than the scripts themselves, don't you think ?

@hasufell
Copy link
Contributor Author

hasufell commented Sep 3, 2020

This is already mentioned in our user guide: https://github.com/input-output-hk/cardano-wallet/wiki/Wallet-command-line-interface

That's only for bash afais.

Also, I'd rather give the command to generate the auto-completion scripts than the scripts themselves, don't you think ?

I think either is fine. I don't have a strong opinion, but it seems the upstream doc suggests to ship the output:
https://github.com/pcapriotti/optparse-applicative/wiki/Bash-Completion#introduction

Normally, the output of --bash-completion-script should be shipped with the program and copied to the appropriate directory (usually /etc/bash_completion.d/) during installation.

Copy link
Member

@Anviking Anviking left a comment

Choose a reason for hiding this comment

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

Personally, I think this makes more sense than having it in the wiki. Nice to have zsh + fish too! 🙏

@rvl
Copy link
Contributor

rvl commented Sep 4, 2020

We should generate the completion scripts for all shells for all programs when building the release archives. This could be done with nix or travis - choose your poison.

That will be more helpful to users than having these scripts in either the git repo or the wiki.

@hasufell
Copy link
Contributor Author

hasufell commented Sep 7, 2020

@rvl are you gonna take a shot at that? I'm not familiar with either of those.

@rvl
Copy link
Contributor

rvl commented Sep 8, 2020

I can coach you if you have time. Travis is just glorified shell scripts embedded in YAML. Nix is just shell scripts embedded in an obscure functional language - start looking in nix/linux-release.nix.

  ```
  cardano-wallet-linux64
  ├── auto-completion
  │   ├── bash
  │   │   └── cardano-wallet.sh
  │   ├── fish
  │   │   └── cardano-wallet.fish
  │   └── zsh
  │       └── _cardano-wallet
  └── cardano-wallet
  ```

  So that users can simply copy-paste them into whatever shell they use.
@KtorZ KtorZ force-pushed the hasufell/shell-completions branch from d0d0f5c to 4ccd3d2 Compare September 28, 2020 13:05
Copy link
Member

@KtorZ KtorZ left a comment

Choose a reason for hiding this comment

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

Discussed in DMs with @hasufell on Slack. Approving my own changes on his behalf.

@KtorZ
Copy link
Member

KtorZ commented Sep 28, 2020

bors merge

iohk-bors bot added a commit that referenced this pull request Sep 28, 2020
2107: Add shell completions r=KtorZ a=hasufell

This is created via optpars-applicative, see:
  https://github.com/pcapriotti/optparse-applicative/wiki/Bash-Completion

Co-authored-by: KtorZ <[email protected]>
@iohk-bors
Copy link
Contributor

iohk-bors bot commented Sep 28, 2020

Build failed:

Failures:

  src/Test/Integration/Scenario/API/Shelley/StakePools.hs:726:9:
  1) API Specifications, SHELLEY_STAKE_POOLS, STAKE_POOLS_LIST_01 - List stake pools, at least one pool eventually produces block
       uncaught exception: IOException of type UserError
       user error (Waited longer than 90s (more than 2 epochs) for an action to resolve. Action: "eventually produces block". Error condition: Just (HUnitFailure (Just (SrcLoc {srcLocPackage = "cardano-wallet-core-integration-2020.9.22-CxOMabAtDjmCg6KD9G8yo3", srcLocModule = "Test.Integration.Scenario.API.Shelley.StakePools", srcLocFile = "src/Test/Integration/Scenario/API/Shelley/StakePools.hs", srcLocStartLine = 734, srcLocStartCol = 17, srcLocEndLine = 734, srcLocEndCol = 49})) (Reason "predicate failed on: 0")))

  To rerun use: --match "/API Specifications/SHELLEY_STAKE_POOLS/STAKE_POOLS_LIST_01 - List stake pools/at least one pool eventually produces block/"

  src/Test/Integration/Scenario/API/Shelley/StakePools.hs:822:5:
  2) API Specifications, SHELLEY_STAKE_POOLS, STAKE_POOLS_GARBAGE_COLLECTION_01 - retired pools are garbage collected on schedule and not before
       uncaught exception: IOException of type UserError
       user error (Waited longer than 90s (more than 2 epochs) for an action to resolve. Action: "Garbage has been collected for epoch 1.". Error condition: Just (HUnitFailure (Just (SrcLoc {srcLocPackage = "cardano-wallet-core-integration-2020.9.22-CxOMabAtDjmCg6KD9G8yo3", srcLocModule = "Test.Integration.Scenario.API.Shelley.StakePools", srcLocFile = "src/Test/Integration/Scenario/API/Shelley/StakePools.hs", srcLocStartLine = 867, srcLocStartCol = 21, srcLocEndLine = 867, srcLocEndCol = 63})) (Reason "predicate failed on: 0")))

  To rerun use: --match "/API Specifications/SHELLEY_STAKE_POOLS/STAKE_POOLS_GARBAGE_COLLECTION_01 - retired pools are garbage collected on schedule and not before/"

Randomized with seed 1431308878

Finished in 2310.5551 seconds
656 examples, 2 failures, 3 pending

@rvl
Copy link
Contributor

rvl commented Sep 30, 2020

Try the integration tests again...

bors r+

@iohk-bors
Copy link
Contributor

iohk-bors bot commented Sep 30, 2020

Build succeeded:

@iohk-bors iohk-bors bot merged commit 6d0f625 into cardano-foundation:master Sep 30, 2020
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