Neptune-Crypto / neptune-core

anonymous peer-to-peer cash
Apache License 2.0
24 stars 7 forks source link

add shell tab completions feature to neptune-cli #44

Closed dan-da closed 11 months ago

dan-da commented 11 months ago

addresses #43

Add completions sub-command that prints out completion code for present shell. Eg in bash do:

neptune-cli completions > ~/.bash_neptune_cli
echo "source ~/.bash_neptune_cli" >> ~/.bashrc

Supports bash, elvish, fish, powershell, zsh

Adds dependency on crate clap_complete.

Notes:

Sword-Smith commented 11 months ago

I tested it out and got it to work on my machine that runs bash. Thank you for this!

One question though: There was a process of manual setup to get the tab-completion to work (the two commands you provide above), and I guess you would have to rerun that command if the CLI interface changes. I'm not much of a Unix/bash expert but is this how it's normally done? Is there a way to avoid this manual step?

Just to clarify, I consider this PR an improvement despite the manual setup process and am happy to merge.

Edit: You already answered the above question in your PR message.

I don't see a need to hide this extra dependency behind a completions feature flag.

We aim to support the big three OSes: Linux, Windown, and MacOS. GitHub's build servers should check whether the new dependency is compatible with those OSes.

dan-da commented 11 months ago

Yeah, I find the present situation with clap completions sub-optimal. Hopefully they will finish up the dynamic completions by the time neptune launches. That provides both more functionality as well as it should always stay current with the binary without any install step. Otherwise, the build.rs method provides a way to generate the shell scripts at compile time without the completions runtime sub-command. They could then be checked in git, installed for end user, etc.

Still, this method was easy to build with few loc and for us devs install can easily be automated with a script like this:

install-bash-completions.sh

LINE='source ~/.bash_neptune_cli '
FILE="$HOME/.bashrc"

cargo run --bin neptune-cli -- completions > ~/.bash_neptune_cli && \
grep -qF -- "$LINE" "$FILE" || echo "$LINE" >> "$FILE"

I can add this script to the PR if you like. Note of course that it is bash specific, so doesn't help for the other supported shells.

Sword-Smith commented 11 months ago

I can add this script to the PR if you like. Note of course that it is bash specific, so doesn't help for the other supported shells.

I'm all in favor of adding scripts to the code base. Please do that.

In case you're wondering what @aszepieniec and I are currently working on, it's the compiler and the standard library that is required to define the consensus logic that must run in the Triton-VM virtual machine.

Once the compiler is where I want it to be, I'll most likely give neptune-core more attention.

dan-da commented 11 months ago

I added the bash install script.

I also took a quick look at what would be involved to use the compile-time generation of completion scripts. It appears that neptune-cli.rs would need to be split into separate files so that the CLI commands can be included from build.rs without redefining main(). Not really a big deal, but then maybe neptune-cli should get its own directory, etc. So that's a road we could pursue if you wish and maybe will happen naturally later on anyway as neptune-cli grows, but I didn't feel right about just diving into it on my own for dubious benefit.

here's a writeup about it. https://kbknapp.dev/shell-completions/

Sword-Smith commented 11 months ago

I approved this PR. Feel free to merge, @dan-da.

dan-da commented 11 months ago

@Sword-Smith I think you will need to merge as I don't have perms.

Only those with write access to this repository can merge pull requests.

Sword-Smith commented 11 months ago

I merged, and then I put the script into a scripts directory. One nice-to-have thing would be if the script reported an error if neptune-core is not running.