Skip to content
This repository has been archived by the owner on Feb 1, 2024. It is now read-only.

Refactor CLI argument parsing in crate_extractor #25

Merged
merged 7 commits into from
Oct 7, 2021

Conversation

m-bock
Copy link
Contributor

@m-bock m-bock commented Oct 7, 2021

  • Updates CLI parsing library "clap" from 2.3 to 3.0

    • In this latest version it's encouraged to use derive macros for parser generation
      The advantage is:
      • A clear rust data structure is defined to contain the CLI arguments
      • parser is fully derived/generated from that
      • Parsing and data usage are separated much better ("Parse, don't validate" principle)
  • Cli argument parsing is outsourced in "cli.rs"

  • Introduced constants for default arguments

  • Makes CI available on all branches

@achimcc achimcc linked an issue Oct 7, 2021 that may be closed by this pull request
4 tasks
Copy link
Contributor

@achimcc achimcc left a comment

Choose a reason for hiding this comment

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

Nice, thanks! This also solves @athei last wish from my previous PR #25, since the latest version of clap seems to suppport the same features as structopt?

@achimcc achimcc merged commit 6980586 into use-ink:main Oct 7, 2021
@athei
Copy link
Contributor

athei commented Oct 8, 2021

Yes. It seems that the new clap version obsoletes structopt.

@m-bock
Copy link
Contributor Author

m-bock commented Oct 8, 2021

@athei
Cool, it's still in beta but worked out really well. Yesterday we tried to find something similar for environment variable parsing. we found envy. Unfortunately it's a bit wordy to define default arguments: https://github.com/paritytech/ink-playground/blob/main/crates/backend/src/env.rs#L17
It's ok, but would be nicer to find something that handles defaults like clap. Do you maybe know about something?

@athei
Copy link
Contributor

athei commented Oct 8, 2021

I think clap supports reading default values from the environment. Why do you need a second crate?

@m-bock
Copy link
Contributor Author

m-bock commented Oct 8, 2021

Oh, we did not see this option. But now I found it.
Thanks for the hint, I think we'll change this!

@m-bock m-bock deleted the refactor branch October 13, 2021 05:46
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CLI Command for .json creation
4 participants