Skip to content
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

CLI refactor #731

Open
5 of 8 tasks
moul opened this issue Apr 11, 2023 · 10 comments
Open
5 of 8 tasks

CLI refactor #731

moul opened this issue Apr 11, 2023 · 10 comments
Assignees
Labels
📦 🌐 tendermint v2 Issues or PRs tm2 related 📦 ⛰️ gno.land Issues or PRs gno.land package related 📦 🤖 gnovm Issues or PRs gnovm related 🌟 improvement performance improvements, refactors ...

Comments

@moul
Copy link
Member

moul commented Apr 11, 2023

Following various discussions, issues, and PRs; I've a proposal for a new (and final?) refactor of the CLI.

Proposed changes:


Read recent discussion at #623
Related with #460
Continues #497
Fixes #729

@grepsuzette
Copy link
Contributor

When a CLI system is mixed with a configuration system (either toml yaml or json...), and can inherit environment variable, I consider it's a very high-level system already.

I suggest to just fork ffcli into our codebase (step 1), and then to see what we do from there, step by step. This would fluidify execution of other tasks.

@moul
Copy link
Member Author

moul commented Apr 14, 2023

I agree, and we can start with just the flag parsing part first.
But it's important to make it consistent and avoid the necessity of doing things like #729.

tbruyelle added a commit to tbruyelle/gno that referenced this issue Apr 19, 2023
This change contains the minimum change required to support some of the requirements described in gnolang#731

What it brings:
- Fork of `ffcli` in `tm2/pkg/commands/ffcli`. Note that the legacy `ff` is still used (so env var and config file override is still supported).
- Accepts flags declared after the command arguments.
This is one of the main complaints about `ffcli`. Now use can run `
-
tbruyelle added a commit to tbruyelle/gno that referenced this issue Apr 19, 2023
This change contains the minimum change required to support some of the requirements described in gnolang#731

What it brings:
- Fork of `ffcli` in `tm2/pkg/commands/ffcli`. Note that the legacy `ff` is still used (so env var and config file override is still supported).
- Accepts flags declared after the command arguments.
This is one of the main complaints about `ffcli`. Now use can run `
-
tbruyelle added a commit to tbruyelle/gno that referenced this issue Apr 19, 2023
This change contains the minimum change required to support some of the requirements described in gnolang#731

What it brings:
- Fork of `ffcli` in `tm2/pkg/commands/ffcli`. Note that the legacy `ff` is still used (so env var and config file override is still supported).
- Accepts flags declared after the command arguments.
This is one of the main complaints about `ffcli`. Now use can run `
-
@ajnavarro ajnavarro added 📦 🤖 gnovm Issues or PRs gnovm related 📦 🌐 tendermint v2 Issues or PRs tm2 related 📦 ⛰️ gno.land Issues or PRs gno.land package related 🌟 improvement performance improvements, refactors ... labels May 19, 2023
@thehowl thehowl self-assigned this May 24, 2023
@moul
Copy link
Member Author

moul commented May 24, 2023

@thehowl checkout this -> https://github.com/rsc/getopt/blob/master/getopt.go

From golang/go#40138

Edit: FYI @thehowl, #828 partially tackled TOML config file support for an existing command.

@albttx
Copy link
Member

albttx commented Jun 15, 2023

With the on-going discussion for running a multi-node testnet, i believe this issue is needed to be linked to it.

Of course, genesis files can be written by hand for the moment, but i believe we should consider to add useful tools to simplify validators works like: (as in cosmos-sdk based chain)

  • gnoland add-genesis-account
  • gnoland gentx
  • gnoland collect-gentx

@moul
Copy link
Member Author

moul commented Jun 22, 2023

Let's start by switching from a single command to multiple subcommands; then we can have PRs for each relevant addition.

Edit: see #937

@thehowl
Copy link
Member

thehowl commented Jul 6, 2023

From discussions:

me: watching through zack's video tutorial on how to build a smart contract: https://youtu.be/F-_dadxcRJM?t=172\
I see that he says that the way to get tokens in a local testnet is by adding your key to the genesis balances file. However, this is in contrast to what we say, for instance, in the Getting Started tutorial we have on the testnets ( https://test3.gno.land/r/demo/boards:testboard/5 ), which instead suggests to use the given mnemonic, which is the corresponding mnemonic for the test1 key in the genesis balances file.
Which one should we suggest to use? A third option could be that of adding a flag to gnoland init to add an address to the genesis block without having to write to a file...

manfred: I'm for adding gnoland init and gnoland mint-token or other helpers
I'm also considering writing a simple script that will look for existing gno wallets on the FS and autoload them when running in dev mode

@notJoon
Copy link
Member

notJoon commented Oct 27, 2023

Looking at the CLI Philosoph paragraph in the PHILOSOPHY.md, It says "No short flags", can I add this feature regardless?

@thehowl
Copy link
Member

thehowl commented Nov 20, 2023

Looking at the CLI Philosoph paragraph in the PHILOSOPHY.md, It says "No short flags", can I add this feature regardless?

This is a conflict I was aware of, although I know informally we verbally said that we should still go in the direction of having (some) short flags.

wdyt @moul ?

@ilgooz
Copy link
Contributor

ilgooz commented Nov 22, 2023

@salmad3 @jeronimoalbi

@moul
Copy link
Member Author

moul commented Dec 7, 2023

I recommend using short flags for common options, particularly if they are widely used in the Go binary or conform to Unix conventions. For example, -v can be used instead of --verbose. We should consider making the addition of short flags rare or even removing --verbose and considering -v as the primary flag instead.

Things to avoid include:

  • Unclear "--help" messages
  • Unclear commands in your history or when sharing them
  • Redundancy in performing tasks (one method is usually better)
  • Discomfort for long-time Unix/Go users

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
📦 🌐 tendermint v2 Issues or PRs tm2 related 📦 ⛰️ gno.land Issues or PRs gno.land package related 📦 🤖 gnovm Issues or PRs gnovm related 🌟 improvement performance improvements, refactors ...
Projects
Status: 🌟 Wanted for Launch
Development

No branches or pull requests

8 participants