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

Ion dump #2

Merged
merged 6 commits into from
Jul 8, 2020
Merged

Ion dump #2

merged 6 commits into from
Jul 8, 2020

Conversation

zslayton
Copy link
Contributor

@zslayton zslayton commented Jul 3, 2020

Description of changes:

  • Running cargo build or cargo install will build the code in the ion-c submodule and link against the resulting libraries.
  • Creates an ion dump command that wraps the ion-c CLI and exposes a limited number of options.
  • Creates a structure that will make it straightforward to add new commands in the future.
  • Adds an "API subject to change" warning to the README.md file.

Testing

If you don't already have it, you can install cargo using rustup.

git clone -b ion-dump https://github.com/amzn/ion-cli.git
cd ion-cli
git submodule update --init --recursive
cargo install --path . 

Example output for ion:

ion 0.1.0
The Ion Team <ion-team@amazon.com>

USAGE:
    ion <SUBCOMMAND>

FLAGS:
    -h, --help       Prints help information
    -V, --version    Prints version information

SUBCOMMANDS:
    dump    Prints Ion in the requested format
    help    Prints this message or the help of the given subcommand(s)

Example output for ion help dump:

ion-dump
Prints Ion in the requested format

USAGE:
    ion dump [OPTIONS] [input]...

FLAGS:
    -h, --help       Prints help information
    -V, --version    Prints version information

OPTIONS:
    -f, --format <format>    Output format [default: pretty]  [values: binary, text, pretty]
    -o, --output <output>    Output file [default: STDOUT]

ARGS:
    <input>...    Input file [default: STDIN]

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

pub mod dump;

// Creates a Vec of CLI configurations for all of the available built-in commands
pub fn built_in_commands() -> Vec<App<'static, 'static>> {
Copy link
Contributor Author

@zslayton zslayton Jul 3, 2020

Choose a reason for hiding this comment

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

The project's directory structure and the contents of this file were inspired by cargo, which follows a similar pattern.

Choose a reason for hiding this comment

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

Are you/cargo making some distinction between "built-in" commands and some other type of commands? If not, perhaps the term is simply "subcommands" (here and elsewhere).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Are you/cargo making some distinction between "built-in" commands and some other type of commands?

Yes. "Built-in" commands are part of the same ion executable (and so show up in the output of ion help) while "external" commands are (potentially 3rd party) executables installed separately on the user's $PATH. See #1 for some extra detail.

Copy link

@pbcornell pbcornell left a comment

Choose a reason for hiding this comment

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

One question below, otherwise looks like a good starting place, and assuming we're comfortable changing the contract as discussions continue (I assume that's what you mean by "API subject to change" in the readme).

pub mod dump;

// Creates a Vec of CLI configurations for all of the available built-in commands
pub fn built_in_commands() -> Vec<App<'static, 'static>> {

Choose a reason for hiding this comment

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

Are you/cargo making some distinction between "built-in" commands and some other type of commands? If not, perhaps the term is simply "subcommands" (here and elsewhere).

@zslayton zslayton merged commit 79aeb06 into master Jul 8, 2020
@zslayton zslayton deleted the ion-dump branch July 8, 2020 22:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants