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: Adapt for 0.12 and 0.13 #8289

Merged
merged 1 commit into from Jul 11, 2016

Conversation

Projects
None yet
6 participants
@roques
Contributor

roques commented Jun 29, 2016

  • separate completion for bitcoind and bitcoin-cli
  • remove RPC support from bitcoind completion
  • add completion for bitcoin-tx and bitcoin-qt

@MarcoFalke MarcoFalke added the Docs label Jun 29, 2016

@MarcoFalke MarcoFalke added this to the 0.13.0 milestone Jun 29, 2016

@jonasschnelli

This comment has been minimized.

Show comment
Hide comment
@jonasschnelli

jonasschnelli Jun 29, 2016

Member

Nice! Thanks.
Concept ACK.

Member

jonasschnelli commented Jun 29, 2016

Nice! Thanks.
Concept ACK.

@MarcoFalke

View changes

Show outdated Hide outdated contrib/bitcoin-tx.bash-completion
@luke-jr

This comment has been minimized.

Show comment
Hide comment
@luke-jr

luke-jr Jul 1, 2016

Member

Concept ACK. Would you mind switching to the standard project copyright header while you're at it?

Member

luke-jr commented Jul 1, 2016

Concept ACK. Would you mind switching to the standard project copyright header while you're at it?

@roques

This comment has been minimized.

Show comment
Hide comment
@roques

roques Jul 1, 2016

Contributor

@luke-jr, I switched to a copyright year range instead of an enumeration, as it is more popular here.
I did compare license headers with several other files in src/ and the license looks identical, so there shouldn't be any legal traps in that regard. However, I don't believe my small contributions over the years make me a "Bitcoin Core Developer" yet.

Contributor

roques commented Jul 1, 2016

@luke-jr, I switched to a copyright year range instead of an enumeration, as it is more popular here.
I did compare license headers with several other files in src/ and the license looks identical, so there shouldn't be any legal traps in that regard. However, I don't believe my small contributions over the years make me a "Bitcoin Core Developer" yet.

@luke-jr

This comment has been minimized.

Show comment
Hide comment
@luke-jr

luke-jr Jul 1, 2016

Member

The "Copyright Bitcoin Core developers" is essentially saying each author retains copyright to their own contributions; there is no centralised/formal/legal entity. Maybe we should clarify this somewhere. (To be clear, I don't consider this a blocker to your PR, just a "nice to have".)

Member

luke-jr commented Jul 1, 2016

The "Copyright Bitcoin Core developers" is essentially saying each author retains copyright to their own contributions; there is no centralised/formal/legal entity. Maybe we should clarify this somewhere. (To be clear, I don't consider this a blocker to your PR, just a "nice to have".)

@MarcoFalke

This comment has been minimized.

Show comment
Hide comment
@MarcoFalke

MarcoFalke Jul 2, 2016

Member

If you are the sole author of the file, it is fine to just put your name and not "Bitcoin Core developers". However, if someone modifies the file, we just put "Bitcoin Core developers" in there to not clutter the headers. The detailed list of authors could then be fetched via git blame or git log or https://github.com/bitcoin/bitcoin/blob/master/doc/release-notes/release-notes-0.10.0.md#credits

Member

MarcoFalke commented Jul 2, 2016

If you are the sole author of the file, it is fine to just put your name and not "Bitcoin Core developers". However, if someone modifies the file, we just put "Bitcoin Core developers" in there to not clutter the headers. The detailed list of authors could then be fetched via git blame or git log or https://github.com/bitcoin/bitcoin/blob/master/doc/release-notes/release-notes-0.10.0.md#credits

@roques

This comment has been minimized.

Show comment
Hide comment
@roques

roques Jul 4, 2016

Contributor

I did some more digging through the Copyright headers. Below src/ the situation is very clear: Either a file originates with Bitcoin and is "Copyright (c) The Bitcoin Core developers" or it originates from an external project and keeps that project's authors' Copyright headers. Satoshi is an exception to that rule. Outside of src/, especially in contrib/devtools/ there are several files which retain their initial authors' Copyright.
I like @luke-jr's interpretation and second the wish for an explicit mention of it somewhere in the developer documentation. — My initial resistance was due to me misunderstanding that I was being asked to assign my copyright to somebody else.

Contributor

roques commented Jul 4, 2016

I did some more digging through the Copyright headers. Below src/ the situation is very clear: Either a file originates with Bitcoin and is "Copyright (c) The Bitcoin Core developers" or it originates from an external project and keeps that project's authors' Copyright headers. Satoshi is an exception to that rule. Outside of src/, especially in contrib/devtools/ there are several files which retain their initial authors' Copyright.
I like @luke-jr's interpretation and second the wish for an explicit mention of it somewhere in the developer documentation. — My initial resistance was due to me misunderstanding that I was being asked to assign my copyright to somebody else.

@MarcoFalke

This comment has been minimized.

Show comment
Hide comment
@MarcoFalke

MarcoFalke Jul 5, 2016

Member

Travis failure unrelated in tests.exe:

Application tried to create a window, but no driver could be loaded.
Make sure that your X server is running and that $DISPLAY is set correctly.
err:systray:initialize_systray Could not create tray window
test count = err:seh:setup_exception stack overflow 3648 bytes in thread 0026 eip 00002aef24a23efb esp 00000000001407c0 stack 0x140000-0x141000-0x340000
Member

MarcoFalke commented Jul 5, 2016

Travis failure unrelated in tests.exe:

Application tried to create a window, but no driver could be loaded.
Make sure that your X server is running and that $DISPLAY is set correctly.
err:systray:initialize_systray Could not create tray window
test count = err:seh:setup_exception stack overflow 3648 bytes in thread 0026 eip 00002aef24a23efb esp 00000000001407c0 stack 0x140000-0x141000-0x340000
@petertodd

This comment has been minimized.

Show comment
Hide comment
@petertodd

petertodd Jul 5, 2016

Contributor

The phrase "Bitcoin Core developers" in the copyright notice just means "developers of Bitcoin Core" - which is anyone at all who has contributed anything. There's a good writeup on this subject that here, which takes into account how distributed revision control systems like git do a good job of recording exact authorship for legal purposes: https://softwarefreedom.org/resources/2012/ManagingCopyrightInformation.html

Concept ACK

Contributor

petertodd commented Jul 5, 2016

The phrase "Bitcoin Core developers" in the copyright notice just means "developers of Bitcoin Core" - which is anyone at all who has contributed anything. There's a good writeup on this subject that here, which takes into account how distributed revision control systems like git do a good job of recording exact authorship for legal purposes: https://softwarefreedom.org/resources/2012/ManagingCopyrightInformation.html

Concept ACK

@laanwj

This comment has been minimized.

Show comment
Hide comment
@laanwj

laanwj Jul 6, 2016

Member

My initial resistance was due to me misunderstanding that I was being asked to assign my copyright to somebody else.

You cannot 'accidentally' assign your copyright to someone else, only by signing an explicit contract. Some projects (such as leveldb) have a separate contributor license agreement that transfers copyright to a company or organization (to Google in that case). Bitcoin Core does not have that.
The only thing you implicitly agree on by contributing is licensing the code under the license of the project (so, MIT ).

Member

laanwj commented Jul 6, 2016

My initial resistance was due to me misunderstanding that I was being asked to assign my copyright to somebody else.

You cannot 'accidentally' assign your copyright to someone else, only by signing an explicit contract. Some projects (such as leveldb) have a separate contributor license agreement that transfers copyright to a company or organization (to Google in that case). Bitcoin Core does not have that.
The only thing you implicitly agree on by contributing is licensing the code under the license of the project (so, MIT ).

@MarcoFalke

This comment has been minimized.

Show comment
Hide comment
@MarcoFalke

MarcoFalke Jul 6, 2016

Member
Member

MarcoFalke commented Jul 6, 2016

@laanwj

This comment has been minimized.

Show comment
Hide comment
@laanwj

laanwj Jul 7, 2016

Member

Anyone knows what is the proper way to test this on fedora with just self-compiled Bitcoin Core?

My guess would be typing this into your shell:

export PATH=$PATH:$BUILDDIR/src
source contrib/bitcoin-tx.bash-completion
source contrib/bitcoind.bash-completion

The commands need to be in your path. The completion script are simply bash scripts that define a function and install it using complete.
But it doesn't seem to work, it complains about not being able to find the command have.

Member

laanwj commented Jul 7, 2016

Anyone knows what is the proper way to test this on fedora with just self-compiled Bitcoin Core?

My guess would be typing this into your shell:

export PATH=$PATH:$BUILDDIR/src
source contrib/bitcoin-tx.bash-completion
source contrib/bitcoind.bash-completion

The commands need to be in your path. The completion script are simply bash scripts that define a function and install it using complete.
But it doesn't seem to work, it complains about not being able to find the command have.

bash-completion: Adapt for 0.12 and 0.13
 * separate completion for bitcoind and bitcoin-cli
 * remove RPC support from bitcoind completion
 * add completion for bitcoin-tx and bitcoin-qt
 * rely on autoloading of completions
@roques

This comment has been minimized.

Show comment
Hide comment
@roques

roques Jul 7, 2016

Contributor

I've looked into testing the completions without installing them and found that the use of have is deprecated. The now updated bash-completion snippets don't do that anymore and when using dh_bash-completion from bash-completion ≥ 1:2.1-4.2 while building the Debian package will be installed in /usr/share/bash-completion/completions/ instead of the deprecated /etc/bash_completion.d/ and autoloaded ondemand.
@MarcoFalke, for you this should mean that when you have Fedora's bash-completion installed and active, you should now be able to simply source the scripts to test them without installing them.

Contributor

roques commented Jul 7, 2016

I've looked into testing the completions without installing them and found that the use of have is deprecated. The now updated bash-completion snippets don't do that anymore and when using dh_bash-completion from bash-completion ≥ 1:2.1-4.2 while building the Debian package will be installed in /usr/share/bash-completion/completions/ instead of the deprecated /etc/bash_completion.d/ and autoloaded ondemand.
@MarcoFalke, for you this should mean that when you have Fedora's bash-completion installed and active, you should now be able to simply source the scripts to test them without installing them.

@MarcoFalke

This comment has been minimized.

Show comment
Hide comment
@MarcoFalke

MarcoFalke Jul 8, 2016

Member

Appears to be working:

$ src/qt/bitcoin-qt -
Display all 101 possibilities? (y or n)
-?                    -datacarrier          -loadblock=           -onlynet=             -rpcallowip=          -torcontrol=
-addnode=             -datacarriersize      -logips               -par=                 -rpcauth=             -torpassword=
-alertnotify=         -datadir=             -logtimestamps        -paytxfee=            -rpcbind=             -txconfirmtarget=
-banscore=            -dbcache=             -maxconnections=      -peerbloomfilters     -rpccookiefile=       -txindex
-bantime=             -debug=               -maxmempool=          -permitbaremultisig   -rpcpassword=         -uacomment=
-bind=                -disablewallet        -maxorphantx=         -pid=                 -rpcport=             -upgradewallet
-blockmaxcost=        -discover             -maxreceivebuffer=    -port=                -rpcthreads=          -usehd
-blockmaxsize=        -dns                  -maxsendbuffer=       -printtoconsole       -rpcuser=             -version
-blockminsize=        -dnsseed              -maxtimeadjustment    -proxy=               -salvagewallet        -wallet=
-blocknotify=         -externalip=          -maxtxfee=            -proxyrandomize       -seednode=            -walletbroadcast
-blockprioritysize=   -fallbackfee=         -maxuploadtarget=     -prune=               -server               -walletnotify=
-bytespersigop        -forcednsseed         -mempoolexpiry=       -reindex              -shrinkdebugfile      -whitebind=
-checkblocks=         -help-debug           -mempoolreplacement   -reindex-chainstate   -spendzeroconfchange  -whitelist=
-checklevel=          -keypool=             -min                  -rescan               -splash               -whitelistforcerelay
-choosedatadir        -lang=                -minrelaytxfee=       -resetguisettings     -sysperms             -whitelistrelay
-conf=                -listen               -mintxfee=            -rest                 -testnet              -zapwallettxes=
-connect=             -listenonion          -onion=               -rootcertificates=    -timeout=             

Though, I couldn't get it work with the cli:

$ source contrib/bitcoin-cli.bash-completion
$ src/bitcoin-cli -regtest g[tab][tab]

shows nothing

Member

MarcoFalke commented Jul 8, 2016

Appears to be working:

$ src/qt/bitcoin-qt -
Display all 101 possibilities? (y or n)
-?                    -datacarrier          -loadblock=           -onlynet=             -rpcallowip=          -torcontrol=
-addnode=             -datacarriersize      -logips               -par=                 -rpcauth=             -torpassword=
-alertnotify=         -datadir=             -logtimestamps        -paytxfee=            -rpcbind=             -txconfirmtarget=
-banscore=            -dbcache=             -maxconnections=      -peerbloomfilters     -rpccookiefile=       -txindex
-bantime=             -debug=               -maxmempool=          -permitbaremultisig   -rpcpassword=         -uacomment=
-bind=                -disablewallet        -maxorphantx=         -pid=                 -rpcport=             -upgradewallet
-blockmaxcost=        -discover             -maxreceivebuffer=    -port=                -rpcthreads=          -usehd
-blockmaxsize=        -dns                  -maxsendbuffer=       -printtoconsole       -rpcuser=             -version
-blockminsize=        -dnsseed              -maxtimeadjustment    -proxy=               -salvagewallet        -wallet=
-blocknotify=         -externalip=          -maxtxfee=            -proxyrandomize       -seednode=            -walletbroadcast
-blockprioritysize=   -fallbackfee=         -maxuploadtarget=     -prune=               -server               -walletnotify=
-bytespersigop        -forcednsseed         -mempoolexpiry=       -reindex              -shrinkdebugfile      -whitebind=
-checkblocks=         -help-debug           -mempoolreplacement   -reindex-chainstate   -spendzeroconfchange  -whitelist=
-checklevel=          -keypool=             -min                  -rescan               -splash               -whitelistforcerelay
-choosedatadir        -lang=                -minrelaytxfee=       -resetguisettings     -sysperms             -whitelistrelay
-conf=                -listen               -mintxfee=            -rest                 -testnet              -zapwallettxes=
-connect=             -listenonion          -onion=               -rootcertificates=    -timeout=             

Though, I couldn't get it work with the cli:

$ source contrib/bitcoin-cli.bash-completion
$ src/bitcoin-cli -regtest g[tab][tab]

shows nothing

@laanwj

This comment has been minimized.

Show comment
Hide comment
@laanwj

laanwj Jul 8, 2016

Member

@MarcoFalke Seems to work here, do you have the daemon running? I think it queries the remote for the command list:

(8289)$ src/bitcoin-cli -regtest ge↹
generate               getaddressesbyaccount  getblockcount          getconnectioncount     getmempoolentry        getnetworkinfo         getrawtransaction      gettxoutproof
generatetoaddress      getbalance             getblockhash           getdifficulty          getmempoolinfo         getnewaddress          getreceivedbyaccount   gettxoutsetinfo
getaccount             getbestblockhash       getblockheader         getinfo                getmininginfo          getpeerinfo            getreceivedbyaddress   getunconfirmedbalance
getaccountaddress      getblock               getblocktemplate       getmempoolancestors    getnettotals           getrawchangeaddress    gettransaction         getwalletinfo
getaddednodeinfo       getblockchaininfo      getchaintips           getmempooldescendants  getnetworkhashps       getrawmempool          gettxout          
Member

laanwj commented Jul 8, 2016

@MarcoFalke Seems to work here, do you have the daemon running? I think it queries the remote for the command list:

(8289)$ src/bitcoin-cli -regtest ge↹
generate               getaddressesbyaccount  getblockcount          getconnectioncount     getmempoolentry        getnetworkinfo         getrawtransaction      gettxoutproof
generatetoaddress      getbalance             getblockhash           getdifficulty          getmempoolinfo         getnewaddress          getreceivedbyaccount   gettxoutsetinfo
getaccount             getbestblockhash       getblockheader         getinfo                getmininginfo          getpeerinfo            getreceivedbyaddress   getunconfirmedbalance
getaccountaddress      getblock               getblocktemplate       getmempoolancestors    getnettotals           getrawchangeaddress    gettransaction         getwalletinfo
getaddednodeinfo       getblockchaininfo      getchaintips           getmempooldescendants  getnetworkhashps       getrawmempool          gettxout          
@laanwj

This comment has been minimized.

Show comment
Hide comment
@laanwj
Member

laanwj commented Jul 8, 2016

ACK 1ba3db6

@MarcoFalke

This comment has been minimized.

Show comment
Hide comment
@MarcoFalke

MarcoFalke Jul 8, 2016

Member

I am not running the daemon. This is good for merge.

Member

MarcoFalke commented Jul 8, 2016

I am not running the daemon. This is good for merge.

@laanwj laanwj merged commit 1ba3db6 into bitcoin:master Jul 11, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

laanwj added a commit that referenced this pull request Jul 11, 2016

Merge #8289: bash-completion: Adapt for 0.12 and 0.13
1ba3db6 bash-completion: Adapt for 0.12 and 0.13 (Christian von Roques)

@str4d str4d referenced this pull request Dec 2, 2016

Merged

Bash completion #1909

zkbot pushed a commit to zcash/zcash that referenced this pull request Dec 8, 2016

zkbot
Auto merge of #1909 - str4d:bash-completion, r=str4d
Bash completion

This PR pulls in bitcoin/bitcoin#8289, updates the bash completion files for use with Zcash, and bundles them into the Debian package.

codablock added a commit to codablock/dash that referenced this pull request Sep 19, 2017

Merge #8289: bash-completion: Adapt for 0.12 and 0.13
1ba3db6 bash-completion: Adapt for 0.12 and 0.13 (Christian von Roques)

codablock added a commit to codablock/dash that referenced this pull request Dec 27, 2017

Merge #8289: bash-completion: Adapt for 0.12 and 0.13
1ba3db6 bash-completion: Adapt for 0.12 and 0.13 (Christian von Roques)

codablock added a commit to codablock/dash that referenced this pull request Dec 28, 2017

Merge #8289: bash-completion: Adapt for 0.12 and 0.13
1ba3db6 bash-completion: Adapt for 0.12 and 0.13 (Christian von Roques)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment