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: Put some common params in a central place #1484

Merged
merged 6 commits into from
Jul 9, 2024
Merged

Conversation

bfops
Copy link
Collaborator

@bfops bfops commented Jul 8, 2024

Description of Changes

Created a crates/cli/src/common_args.rs file that currently just contains the most common form of the --server/-s param and the --identity/-i param.

API and ABI breaking changes

None

Expected complexity level and risk

1

Testing

  • CI testing
  • Check that help text has been unaffected
    • Confirmed:
$ spacetime energy set-balance --help
Update the current budget balance for a database

Usage: spacetime energy set-balance [OPTIONS] <balance> [identity]

Arguments:
  <balance>
          The balance value to set

  [identity]
          The identity to set a balance for. If no identity is provided, the default one will be used.

Options:
  -s, --server <server>
          The nickname, host name or URL of the server on which to update the identity's balance

  -q, --quiet
          Runs command in silent mode

  -h, --help
          Print help (see a summary with '-h')

@bfops bfops changed the title CLI: Put common params in a central place CLI: Put some common params in a central place Jul 8, 2024
@bfops bfops added the CLI only This change only affects the CLI behavior label Jul 8, 2024
@bfops bfops marked this pull request as ready for review July 8, 2024 18:43
@bfops bfops requested a review from jdetter July 8, 2024 18:44
Arg::new("server")
.long("server")
.short('s')
.help("The nickname, host name or URL of the server")
Copy link
Collaborator

@jdetter jdetter Jul 8, 2024

Choose a reason for hiding this comment

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

if you do .help multiple times, does that latter one take precedence? For example, below you're doing something like server().help("custom help section for this command"). Does this work?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'm testing this rn

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

confirmed this works as desired (see Testing section)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Thanks, ideally it would just work and save us all a lot of suffering 😅

Copy link
Collaborator

@jdetter jdetter left a comment

Choose a reason for hiding this comment

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

Q on this, will properly review later

@bfops bfops enabled auto-merge July 8, 2024 19:03
@@ -0,0 +1,15 @@
use clap::Arg;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

nit @jdetter does this belong more in src/ or src/subcommands?

Copy link
Collaborator

Choose a reason for hiding this comment

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

This is fine, leave it as is 👍

Copy link
Collaborator

@jdetter jdetter left a comment

Choose a reason for hiding this comment

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

Works for me, thanks Zeke 👍

@@ -0,0 +1,15 @@
use clap::Arg;
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is fine, leave it as is 👍

@@ -27,16 +28,9 @@ pub fn cli() -> clap::Command {
.help("The name of the reducer to call"),
)
.arg(Arg::new("arguments").help("arguments formatted as JSON").num_args(1..))
.arg(common_args::server().help("The nickname, host name or URL of the server hosting the database"))
Copy link
Collaborator

Choose a reason for hiding this comment

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

the overrides appear to be working

boppy@boppy-macbook SpacetimeDB % spacetime call --help
Invokes a reducer function in a database

Usage: spacetime call [OPTIONS] <database> <reducer_name> [arguments]...

Arguments:
  <database>      The database domain or address to use to invoke the call
  <reducer_name>  The name of the reducer to call
  [arguments]...  arguments formatted as JSON

Options:
  -s, --server <server>      The nickname, host name or URL of the server hosting the database
  -i, --identity <identity>  The identity to use for the call
  -a, --anon-identity        If this flag is present, the call will be executed with no identity provided
  -h, --help                 Print help

Run `spacetime help call` for more detailed information.

@bfops bfops added this pull request to the merge queue Jul 9, 2024
Merged via the queue into master with commit e913e8c Jul 9, 2024
6 checks passed
lcodes pushed a commit that referenced this pull request Jul 9, 2024
Co-authored-by: Zeke Foppa <github.com/bfops>
@bfops bfops deleted the bfops/common-cli-params branch July 9, 2024 17:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLI only This change only affects the CLI behavior
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants