Skip to content
This repository has been archived by the owner on Oct 4, 2024. It is now read-only.

add yarg’s strict() to chain. add walletUrl, contractName as options #265

Merged
merged 10 commits into from
Mar 31, 2020
Merged

add yarg’s strict() to chain. add walletUrl, contractName as options #265

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
219aa28
add yarg’s strict() to chain. add walletUrl, contractName as options
mikedotexe Feb 6, 2020
6d4dece
add hidden key to walletUrl and contractName
mikedotexe Feb 6, 2020
efc12ac
Merge branch 'master' into add-config-vars-as-args
mikedotexe Mar 31, 2020
7d35f56
add masterAccount option in near-cli
mikedotexe Mar 31, 2020
3189a4e
fix test using delete_account to delete
mikedotexe Mar 31, 2020
ae31dc2
fix test using defult to ci networkId
mikedotexe Mar 31, 2020
6e2d01f
fix lint spacing
mikedotexe Mar 31, 2020
bd58809
remove node 10 from travis
mikedotexe Mar 31, 2020
dcf0d8a
fix typo for masterAccount description
mikedotexe Mar 31, 2020
bc7a73f
in test_generate_key, use $NODE_ENV as flag and directory
mikedotexe Mar 31, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion .travis.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
language: node_js
node_js:
- 10
- 12
env:
- NODE_ENV=ci
Expand Down
18 changes: 18 additions & 0 deletions bin/near-cli.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,7 @@ const clean = {

let config = require('../get-config')();
yargs // eslint-disable-line
.strict()
.middleware(require('../utils/check-version'))
.scriptName('near')
.option('nodeUrl', {
Expand All @@ -170,6 +171,22 @@ yargs // eslint-disable-line
desc: 'Unique identifier for the account',
type: 'string',
})

.option('walletUrl', {
desc: 'Website for NEAR Wallet',
type: 'string',
hidden: true
})
.option('contractName', {
desc: 'Account name of contract',
type: 'string',
hidden: true
})
.option('masterAccount', {
desc: 'Master account used when creating new accounts',
type: 'string',
hidden: true
Copy link
Contributor

Choose a reason for hiding this comment

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

Why hide all of these?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a good debate to be had. The short answer is, there's a long road to getting near-shell consistent, and this is something of a bandaid. Actually, this whole PR is a bandaid.
Note that when you run: near to display the default help from yargs, it spits out commands and then the Options: section. These are flags that typically (again, we're working on it) apply to the top-level commands. Actually, the only good example of this is --accountId here.
Then, when you run near create_account --help you'll see the relevant Options: for that particular command, including --masterAccount.
This hidden key is not hiding it everywhere, but hiding it from the top-level list. I'm not against removing the hidden part, but it'll just make th

Copy link
Contributor Author

Choose a reason for hiding this comment

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

whoops. It'll just make that a bit longer. The fundamental issue here is we want to use strict mode, but we can't do that when we load config.js and it contains additional flags that aren't captured at the top level. So in this PR we're like, "yeah, you can give me a masterAccount flag here, it won't do much on its own, but you can"
And from there yargs's strict mode is possible whereby it'll scream at you if you misspell something or provide suggestions (recommendCommands) or tell you what you're missing.

Copy link
Contributor

Choose a reason for hiding this comment

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

Excellent explanation, thank you

})
.middleware(require('../middleware/print-options'))
.middleware(require('../middleware/key-store'))
.command(require('../commands/create-account'))
Expand Down Expand Up @@ -198,6 +215,7 @@ yargs // eslint-disable-line
'outDir': 'out_dir'
})
.showHelpOnFail(true)
.recommendCommands()
.demandCommand(1, chalk`Pass {bold --help} to see all available commands and options.`)
.usage(chalk`Usage: {bold $0 <command> [options]}`)
.epilogue(chalk`Check out our epic whiteboard series: {bold http://near.ai/wbs}`)
Expand Down
2 changes: 1 addition & 1 deletion test/test_account_operations.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,4 +17,4 @@ if [[ ! "$RESULT" =~ $EXPECTED ]]; then
exit 1
fi

../bin/near delete_account $testaccount test.near
../bin/near delete $testaccount test.near
mikedotexe marked this conversation as resolved.
Show resolved Hide resolved
6 changes: 3 additions & 3 deletions test/test_generate_key.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@

#!/bin/bash
set -ex
KEY_FILE=neardev/default/generate-key-test.json
KEY_FILE=neardev/$NODE_ENV/generate-key-test.json
rm -f "$KEY_FILE"

RESULT=$(./bin/near generate-key generate-key-test --networkId default)
RESULT=$(./bin/near generate-key generate-key-test --networkId $NODE_ENV)
echo $RESULT

if [[ ! -f "${KEY_FILE}" ]]; then
Expand All @@ -18,7 +18,7 @@ if [[ ! "$RESULT" =~ $EXPECTED ]]; then
exit 1
fi

RESULT2=$(./bin/near generate-key generate-key-test --networkId default)
RESULT2=$(./bin/near generate-key generate-key-test --networkId $NODE_ENV)
echo $RESULT2

EXPECTED2=".*Account has existing key pair with ed25519:.+ public key.*"
Expand Down