The completions are dynamically generated from the respective binary help pages.
Completions should be sourced into the shell or added to
$XDG_CONFIG/fish/completions. See where to put completions for more information.
As the completions are auto-generated they should only require as much maintenance as the bash equivalents, which is to say very little!
Concept ACK b4db43a
Tested using Fish Version 3.3.1 on FreeBSD 13.0 RELEASE I added the completion files to ~/.config/fish/completions Worked as expected and was able to autocomplete all command arguments.
contrib/fish-completion subdirectory (or perhaps contrib/completion/{bash,fish} and move the bash-completion too)
contrib/completion/{bash,fish} makes the most sense to me.
I realise we don’t have this for the existing bash-completion scripts, but I think we should probably have these in a new
contrib/fish-completionsubdirectory (or perhapscontrib/completion/{bash,fish}and move the bash-completion too)
Also agree with this.
Unless there are any strong objections I will retouch with completions in subfolders.
Unless there are any strong objections I will retouch with completions in subfolders.
I think that’s fine to do.
As the completions are auto-generated they should only require as much maintenance as the bash equivalents, which is to say very little!
That’s good at least.
Is there any reason you’ve excluded the other utilities?
You could add yourself to the REVIEWERS file for contrib/completion/*.
I think that’s fine to do.
Great I will re-arrange like this then.
Is there any reason you’ve excluded the other utilities?
Which utilities to do you mean in this context? I included bitcoind, bitcoin-cli and bitcoin-tx here.
You could add yourself to the REVIEWERS file for
contrib/completion/*.
Sure, ok
Which utilities to do you mean in this context? I included bitcoind, bitcoin-cli and bitcoin-tx here.
We also have bitcoin-wallet, bitcoin-util.
Which utilities to do you mean in this context? I included bitcoind, bitcoin-cli and bitcoin-tx here.
We also have
bitcoin-wallet,bitcoin-util.
Ah right. I can certainly add those. I was just starting by essentially mirroring what we have in bash, but very happy to give fish superior completion support :)
I will add it into the fish completions commit.
Strong Concept ACK!
Thanks for doing this. I use the bash completions so I haven’t tested this locally, but a big fan of having more autocomplete options, as it’s a big usability improvement for the tools.
One suggestion regarding commit hygiene (feel free to ignore): prefix the commits with the relevant tags, e.g refactor: ... for the first, scripts: ... for the second, and doc: ... for the third
Concept ACK, and agree the new directory structure is better.
Edit: might want to add a small mention to contrib/README.md of the completions directory.
Concept ACK
paging @willcl-ark; this needs updating, and I want to get fishy 🐟
Concept ACK
paging @willcl-ark; this needs updating, and I want to get fishy 🐟
Ping received! I’ll update and finish this ASAP.
Finally updated, sorry about the delay.
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
See the guideline for information on the review process.
If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.
No conflicts as of last run.
bitcoin-cli help <tab> and get completions, which is very helpful. doesn’t seem to work with the fish completions but im guessing it’s a fairly easy fix?
while testing, i noticed that the bash completions allow you to do
bitcoin-cli help <tab>and get completions, which is very helpful. doesn’t seem to work with the fish completions but im guessing it’s a fairly easy fix?
Thanks for the nudge.
Yes this should simply require adding help as a subcommand, which I shall try shortly.
bitcoin-cli help <tab> in db85a5214894e813d4d7aad3dcf28dc19c2aa293 so that it shows completions.
ACK https://github.com/bitcoin/bitcoin/pull/24611/commits/db85a5214894e813d4d7aad3dcf28dc19c2aa293
tested by adding files to ~/.config/fish/completions/ and trying bitcoin-cli <tab> and bitcoin-cli help <tab>. works great, thanks for adding these! using bitcoin-cli help <tab> is a really great way to quickly check the docs on commands, making bitcoin-cli much more usable
Concept ACK
I use fish and would like to have completions. However the implementation here has some issues, although not necessarily blocking ones.
bitcoind of bitcoin-qt -server is not running, then the autocompletion for bitcoin-cli shows error: and Make as autocompletions. These are the first words of the error it gives when the RPC server is not found. They should probably be excluded from the completion.-help-debug options are missing. While these are debug options, some commonly used ones such as -regtest are part of them and I expect that the only people who would be using these completions are developers who also use those debug options. So it might make sense to include them as well.bitcoin-qt should have completions 0> src/bitcoind ~/.config/fish/completions/bitcoind.fish (line 1): Unknown command. '/home/andy/.pyenv/libexec/pyenv/bitcoind' exists but is not an executable file.
1bitcoind --help | sed -n '/^ -/p' | sed -e 's/ -/-/;s/=.*/=/' | sed -n '/\?$/!p'
2^
3in command substitution
4 called on line 5 of file ~/.config/fish/completions/bitcoind.fish
5from sourcing file ~/.config/fish/completions/bitcoind.fish
6~/.config/fish/completions/bitcoind.fish (line 5): Command not executable
7set --local options (bitcoind --help | sed -n '/^ -/p' | sed -e 's/ -/-/;s/=.*/=/' | sed -n '/\?$/!p')
8 ^
9from sourcing file ~/.config/fish/completions/bitcoind.fish
10complete --no-files bitcoind
=) put a space after the option which requires an additional backspace in order to be able to set the parameter.
Thanks for the ping on this, I will fix and rework it as suggested.
A thought I had; might it be more useful (and faster runtime) to have a generate_fish_completions.fish script in this repo, which you could run once to generate completions once and install them wherever you choose? This way too we don’t poll e.g. bitcoin-cli help with each tab on autocomplete? You could just re-run the script after you compile, if you want to update completions.
It would want to be run with bitcoind/bitcoin-qt running so that it generated all the correct completions, but other than that it would work better IMO, except from the fact that it would not be dynamic w.r.t. the current version of bitcoin installed automatically.
I haven’t personally noticed any runtime performance issue tbh, but thinking about how to fix the error and Make autocompletions you mention above made me wonder if the above approach might be a little better…
Status:
If either mainnet bitcoind of bitcoin-qt -server is not running, then the autocompletion for bitcoin-cli shows error: and Make as autocompletions. These are the first words of the error it gives when the RPC server is not found. They should probably be excluded from the completion.
Note: This might be harder to fix than it seems, as it appears to me that fish autocomplete is calling bitcoin-cli in the background regardless of how I gate the call inside the completion file itself, still investigating further though.
The -help-debug options are missing. While these are debug options, some commonly used ones such as -regtest are part of them and I expect that the only people who would be using these completions are developers who also use those debug options. So it might make sense to include them as well.
bitcoin-qt should have completions
Attempting to get a completion with the relative path to the binary results in an error @achow101 do you mean that you are trying to get completions from e.g. a binary in a build dir? If so I am unsure that this will be easily possible without asking the user to remove all completions first (`complete -c binary_name -e) and then re-sourcing the completion file. And even then, there would need to be some logic to check CWD and see if we dynamically generate for binary in current dir, or on PATH. Even if we got all that working, user would need to re-generate each time they wanted to switch between a local dir binary and binary on PATH (to the best of my knowledge)…
Completions that require user provided parameter (those that show up as ending with a =) put a space after the option which requires an additional backspace in order to be able to set the parameter.
do you mean that you are trying to get completions from e.g. a binary in a build dir?
Yes
If so I am unsure that this will be easily possible without asking the user to remove all completions first (`complete -c binary_name -e) and then re-sourcing the completion file. And even then, there would need to be some logic to check CWD and see if we dynamically generate for binary in current dir, or on PATH. Even if we got all that working, user would need to re-generate each time they wanted to switch between a local dir binary and binary on PATH (to the best of my knowledge)…
It doesn’t necessarily need to work with relative paths, it just can’t be throwing an error.
Hey @achow101 I think this is ready for a re-test now as I’ve addressed all your previous comments :)
It should work with relative binaries, as well as not throw any errors if the RPC backend is offline. I also changed the style of the completions to use fish functions so that completions are always dynamically loaded against the current commandline contents which avoids vachine potentially stale completions as it was before.
ACK 803223377c85668a80946a5d5bd22decd1bfd6a4
Tested manually and completions appear to be as expected.
Sorry to change after an ACK, but I’ve found that this was swallowing help completions. This fix to only use the first token from the commandline in 15aafd3 fixes this for me.
git range-diff 8032233...15aafd3ac
There appear to be command completions for bitcoin-cli even if the command already has an RPC in it.
I also noticed this but it seems to only happen for RPC commands which also take arguments. For example:
0bitcoin-cli abortrescan <TAB>
does nothing (expected behavior).
However:
0bitcoin-cli abandontransaction <TAB>
brings up the autocomplete list again. I also tested bitcoin-cli help <TAB> and this works as expected
Move bash completions to
contrib/completions/bash/*
Precursor to adding fish completions
Completions are dynamically generated from the respective binary help
pages.
Completions should be sourced into the shell or added to
$XDG_CONFIG/fish/completions.
ACK https://github.com/bitcoin/bitcoin/commit/ccba4fe7e3113f5d3bb2fbe54147b3a17e1bfa24
manually tested, both bitcoin-cli <TAB> and bitcoin-cli help <TAB> bring up the options and verified that bitcoin-cli command <TAB> doesn’t keep bringing up the autocomplete.