refactor: git-like usage commands #338

tomyrd · 2024-05-13T18:52:06Z

closes #333

The previous command structure was tree-like with sub-commands hidden inside other sub-commands. This PR looks to change the structure to be more git-like with a lot of top level commands with flags that modify their functionality.

Some screenshots provided by @mFragaBA:

igamigo · 2024-05-13T20:18:46Z

Looks good so far! I am personally not sure if I prefer the flatter structure versus the more subcommand-based one. It seems a bit more usable/cleaner at first sight, but also it showcases the types of entities that are involved a little less. But in any case, here are some notes of what I reviewed so far if we decide to go this route:

I'd change --acc-type to --type in the new-account command, since it feels a bit redundant.
I think I'd change the "Config updated..." to "Set the default account to {account_id}" (and "Removed the default account" for unsetting it) or something similar
I think --default none is not ideal but I couldn't think of anything better
We'd have to be careful with the docs (both here and in miden-base)

bobbinth · 2024-05-13T20:25:15Z

It seems a bit more usable/cleaner at first sight, but also it showcases the types of entities that are involved a little less.

Agreed - but I think we can showcase the entities via the client library interface and CLI could still be a bit more "customized" to make it more user friendly.

I'd change --acc-type to --type in the new-account command, since it feels a bit redundant.

As I mentioned in #333 (comment), I'd probably split this into two commands new-wallet and new-faucet. This way, both commands would be simpler (in terms of options to chose from).

mFragaBA

Nice work! Left a few comments

src/cli/export.rs

src/cli/import.rs

src/cli/new_account.rs

src/cli/notes.rs

src/cli/mod.rs

src/cli/new_account.rs

mFragaBA

It wasn't changed in this PR but I noticed that for the init command we show:

The user isn't asked for an input if --store-path isn't provided.

Can we fix that?

src/cli/notes.rs

src/cli/account.rs

igamigo · 2024-05-15T17:25:25Z

CHANGELOG.md

@@ -1,5 +1,6 @@
 # Changelog

+* Flatten the cli subcommand tree.


Suggested change

* Flatten the cli subcommand tree.

* Flatten the CLI subcommand tree.

mFragaBA

send has two options that use -f as the shortened version (--force, --faucet)

src/cli/notes.rs

mFragaBA

LGTM!

* feat: Integrate next changes * fix: Back to next * Fix conflicts * Use node next * fix: Tetss * refactor: reuse function to create mocked client * fix: update swap note script root * fix: Generalize Authenticator * Merge fixes and refactors * Lints * docs: update docs from changes to CLI (#336) * feat: Utilize expected notes from `ExecutedTransaction` (#329) * feat: Use expected notes from TransactionResult * fix: Re-add output note check * Remove unused code * Lints * Fix tests * fix: Tetss * Stash * fix: Address reviews * fix: Tests * fix: Lints * fix: Lints * Try fix test * fix: Merge conflicts * Fix tests * docs: Address reviews * Lints * fix: New script root * Use 0.3 * fix: Correct genesis toml file * fix: test fix con cargo make start-node * fix: temp fix to rename config paths * fix: Correct miden-node toml * fix: Ship our own node config files * Migrate to node 0.3 * feat: Use incoming MMR proof to add block authentications for notes created in past blocks (#337) * test: Integration tests run with main node * fix: Remove std::cmp::max * refactor: git-like usage commands (#338) * Move new account creation to command * Flatten account command * Refactor `default` option * Separate new account commands * Refactor `notes` command * Refactor import comand to infer type * Remove intermediate sucommand for export * Flatten tags command * Separate tx new commands * Fix documentation * Join p2id and p2idr to send command * Remove trait * Change comment suggestions * Clarify new account help comments * Ask for storage path if not specified * Fixed problem with listing notes with no filter * Added a message for default account updates * Change suggestions * Remove prompt for store path * Added all NotesFilter * Fix merge * feat: enable swap notes (#324) * feature: add swap tx request and store swap payback note * feature: insert input note record from payback note details * feature: show message to user on how to track payback notes * fix: integration test compilation * make swap tx send the offered asset * fix: temporary fix for partial output notes * test: add skeleton for integration test * fix: lint issues * test: add test for onchain swap flow * test: add test for offchain swap flow * test: fix note minting for swap tests * test: fix compilation errors * fix: use all committed note ids to check transactions to commit * fix: fix tests checks * address review comments * docs: update CHANGELOG * docs: update CLI reference * address remaining comment * fix: clippy issues * fix: remove wrong check * refactor: store partial notes in relevant notes of transaction result * rephrase doc comment Co-authored-by: igamigo <[email protected]> * address review comments * fix: fix offchain swap test --------- Co-authored-by: igamigo <[email protected]> * docs: fix doc comments for swap clap command * feat: update cli-reference with new command structure (#342) * Update cli-reference with new command structure * Fix merge * Fix merge * Update cli-reference.md --------- Co-authored-by: igamigo <[email protected]> * Update README.md * CLI config.md --------- Co-authored-by: Martin Fraga <[email protected]> Co-authored-by: Martin Fraga <[email protected]> Co-authored-by: Tomas Rodriguez Dala <[email protected]>

tomyrd changed the base branch from main to igamigo-next-0.3 May 13, 2024 18:52

tomyrd mentioned this pull request May 13, 2024

Refactor commands in favor of git-like usage #333

Closed

tomyrd added 4 commits May 14, 2024 11:46

Move new account creation to command

eb35eb3

Flatten account command

84f475f

Refactor default option

041802a

Separate new account commands

d7fbe1f

tomyrd force-pushed the tomyrd-command-refactor branch from 34f554d to d7fbe1f Compare May 14, 2024 14:48

tomyrd added 8 commits May 14, 2024 12:04

Refactor notes command

72960c0

Refactor import comand to infer type

323bbe5

Remove intermediate sucommand for export

4e88699

Flatten tags command

c805cca

Separate tx new commands

5e09088

Fix documentation

7e79969

Join p2id and p2idr to send command

72c52f9

Remove trait

be31b4b

mFragaBA reviewed May 15, 2024

View reviewed changes

Change comment suggestions

a6f9ce5

mFragaBA reviewed May 15, 2024

View reviewed changes

src/cli/new_account.rs Outdated Show resolved Hide resolved

mFragaBA reviewed May 15, 2024

View reviewed changes

Clarify new account help comments

92ab2d8

mFragaBA reviewed May 15, 2024

View reviewed changes

src/cli/notes.rs Outdated Show resolved Hide resolved

mFragaBA reviewed May 15, 2024

View reviewed changes

src/cli/account.rs Outdated Show resolved Hide resolved

igamigo reviewed May 15, 2024

View reviewed changes

mFragaBA reviewed May 15, 2024

View reviewed changes

tomyrd added 4 commits May 15, 2024 14:56

Ask for storage path if not specified

f633fd1

Fixed problem with listing notes with no filter

cb80ea7

Added a message for default account updates

b74112e

Change suggestions

0936fb9

Remove prompt for store path

b53c53b

mFragaBA reviewed May 15, 2024

View reviewed changes

src/cli/notes.rs Show resolved Hide resolved

tomyrd added 2 commits May 15, 2024 17:04

Added all NotesFilter

7f7cc7b

Merge branch 'igamigo-next-0.3' into tomyrd-command-refactor

567f724

mFragaBA approved these changes May 15, 2024

View reviewed changes

Fix merge

a7cd13b

tomyrd marked this pull request as ready for review May 15, 2024 20:22

igamigo merged commit b986b56 into igamigo-next-0.3 May 15, 2024
6 checks passed

igamigo deleted the tomyrd-command-refactor branch May 15, 2024 20:43

igamigo mentioned this pull request May 15, 2024

feat: Integrate newer changes #321

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor: git-like usage commands #338

refactor: git-like usage commands #338

tomyrd commented May 13, 2024 •

edited

Loading

igamigo commented May 13, 2024 •

edited

Loading

bobbinth commented May 13, 2024

mFragaBA left a comment

mFragaBA left a comment

igamigo May 15, 2024

tomyrd May 15, 2024

mFragaBA left a comment

mFragaBA left a comment

		@@ -1,5 +1,6 @@
		# Changelog

		* Flatten the cli subcommand tree.

refactor: git-like usage commands #338

refactor: git-like usage commands #338

Conversation

tomyrd commented May 13, 2024 • edited Loading

igamigo commented May 13, 2024 • edited Loading

bobbinth commented May 13, 2024

mFragaBA left a comment

Choose a reason for hiding this comment

mFragaBA left a comment

Choose a reason for hiding this comment

igamigo May 15, 2024

Choose a reason for hiding this comment

tomyrd May 15, 2024

Choose a reason for hiding this comment

mFragaBA left a comment

Choose a reason for hiding this comment

mFragaBA left a comment

Choose a reason for hiding this comment

tomyrd commented May 13, 2024 •

edited

Loading

igamigo commented May 13, 2024 •

edited

Loading