New commands layout #75
Conversation
Co-authored-by: Vlad Frolov <frolvlad@gmail.com>
Co-authored-by: Vlad Frolov <frolvlad@gmail.com>
Co-authored-by: Benjamin Kurrek <57506486+BenKurrek@users.noreply.github.com>
|
I believe I have addressed all the comments except the configuration (I feel it is not a blocker at the moment) |
| ``` | ||
| contract | ||
| - call-function | ||
| - as-read-only <account-id> <function-name> <function-args> network <"mainnet"|"testnet"|...> <now|at-timestamp|at-block-height|at-block-hash> |
There was a problem hiding this comment.
Why not just view? Even knowing that as-read-only means view I found it confusing and is less intuitive since we use view everywhere else.
near-cli contract call-function as-read-only ...
Is also a bit long. I know you're love for being explicit, but having some aliases here would be nice.
near contract view
As a short cut would be very handy.
There was a problem hiding this comment.
To me just saying view might lead some people to think that they're viewing the contract code rather than it being viewing the result of a read-only function.
There was a problem hiding this comment.
If you execute a view call and try to invoke a host function like creating a transaction, you will get an error that says it was banned in a view call. So my point is that we use view in our documentation and baked into the runtime, so using it, especially in this context helps people to learn it.
There was a problem hiding this comment.
@willemneal If you finish up the whole command, you will see that it does not make any sense to save a few characters:
near-cli contract view zavodil.poolv1.near get_owner_id '{}' network mainnet now
near-cli contract call-function as-read-only zavodil.poolv1.near get_owner_id '{}' network mainnet now
I don't expect anyone to type it out manually as we have interactive mode that guides users through it, and prints out the final command if you want to re-use it later.
contract view makes sense only to victims of near-cli-js naming. Anyone else will be confused and guess that this command will show you the contract, not calling a function. Yet, the good part of this new approach is that you will be able to implement your own extension and call it as near-cli view ... just fine, if you feel you want to have a shortcut.
|
@volovyk-s @BenKurrek @khorolets @miraclx @willemneal I feel this version is good to go and we can merge it now, and iterate on "login" commands, configs, and other nits in separate PRs. Any objections? |
|
My only nit left and it's fine if everyone else thinks otherwise, but
I'll go ahead and approve, but just wanted to give one more argument for not removing view. |
I'm fine with this as well - I don't have a strong opinion as to which I prefer. I'm fine with both. |
|
So far I feel ok with going I forgot to add commands for offline transaction construction options (in order to construct a transaction you need to specify recent block hash and access key nonce), so I will finish that up before merging this PR. |
| - transaction signature options here (see below) | ||
| - print-to-terminal network <"mainnet"|"testnet"|...> | ||
| - transaction signature options here (see below) | ||
| - use-manually-provided-seed-phrase "twelve words goes here" network <"mainnet"|"testnet"|...> |
There was a problem hiding this comment.
This is an issue I filled with near-cli. It's bad security practice to use a seed phrase as a CLI arg. So perhaps this should be interactive like a password.
There was a problem hiding this comment.
It is both interactive and non-interactive, and again, I don’t expect users to type the command out manually most of the time, so the parameters are there only for reproducibility and integration within scripts, which I don’t see a reason to limit to use params
|
FYI, This new design is already mostly implemented in a separate repo: https://github.com/FroVolod/near-cli-rs-instance/tree/without-generics |
|
Just an update: #115 implements all the base CLI commands layout |
|
@volovyks @ChaoticTempest Thanks for all your valuable input along the way! Check out the 0.2.0-pre.1 release! |
Direct link to the rendered doc