search

trufflesuite / ganache

:warning: The Truffle Suite is being sunset. For information on ongoing support, migration options and FAQs, visit the Consensys blog. Thank you for all the support over the years.
https://consensys.io/blog/consensys-announces-the-sunset-of-truffle-and-ganache-and-new-hardhat?utm_source=github&utm_medium=referral&utm_campaign=2023_Sep_truffle-sunset-2023_announcement_
MIT License
2.62k stars 681 forks source link

CLI option aliases for `--wallet.accounts` are confusing #2575

Open MicaiahReid opened 2 years ago

MicaiahReid commented 2 years ago

The option --wallet.accounts has alias --account, but not --accounts. This was confusing to me. Maybe I'm dumb. Maybe it would help our users to have both aliases.

davidmurdoch commented 2 years ago

The reason for this is that on the command line you can't provide a JavaScript array of accounts; you have to provide multiple account options, e.g., ganache --wallet.account "0xkey,balance" --wallet.account "0xkey,balance"

MicaiahReid commented 2 years ago

@davidmurdoch But ganache --wallet.account "0xkey,balance" fails, it has to be --wallet.accounts. So maybe the answer isn't to add the --accounts alias but to change --wallet.accounts to --wallet.account (but that would be a breaking change so not ideal).

davidmurdoch commented 2 years ago

Ohhh, I get it now. Hmmm, yeah, the alias should exist in both locations.

amandesai01 commented 2 years ago

Maybe we can update alias to --wallet.account and keep --wallet.accounts but show a warning when used. And after sometime deprecate it? I am willing to create a PR to do this, or any other suggestion. Let me know.

MicaiahReid commented 2 years ago

@amandesai01 Thanks for offering! A PR would most definitely be welcome. We discussed and decided we want both the --accounts and --wallet.account aliases added. No warning necessary. Let us know if you need any help! A good starting point is the wallet-options.ts file.

amandesai01 commented 2 years ago

@MicaiahReid can you elaborate? Do you want all options to do same thing? I mean --accounts, --wallet.account, --wallet.accounts and --account all have to do same thing?

MicaiahReid commented 2 years ago

@amandesai01 Yes, exactly. --account, --accounts, and --wallet.account would all be aliases for --wallet.accounts, so they'd all do the same thing.

@davidmurdoch in looking into this I also found that --accounts is already an alias for --wallet.totalAccounts 😱. I think we should make --wallet.totalAccounts only have the alias --totalAccounts as part of this change..

davidmurdoch commented 2 years ago

We can't change --accounts to not be an alias of --wallet.totalAccounts unless we want to release a v8.0 :sob:

The --accounts option is that way because that is what the option was in v6, and we designed v7 to have no breaking changes for command line use to make it easier to upgrade to, since there were already so many other breaking changes in the 7.0 release.

MicaiahReid commented 2 years ago

I see. Any thoughts on how we should proceed? Save this as an issue for an 8.0 release and use it to clean up all CLI options in general?

davidmurdoch commented 2 years ago

I guess we either need to save it for 8.0 and/or rename (with a backward compatible alias) --wallet.accounts to something more explicit, like --wallet.initialAccounts, with a top-level alias of --initialAccounts? :-/