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

proposal: Implement serde::Serializer and Source/AsyncSource for Value #315

Closed
Xuanwo opened this issue Apr 15, 2022 · 2 comments
Closed

Comments

@Xuanwo
Copy link
Contributor

Xuanwo commented Apr 15, 2022

I'm working on an app that uses clap for command line args parsing. I want to support loading configs in various ways with the specific rules:

  • loading from env
  • loading from config files
  • loading from args

To avoid duplicate code, I'm trying to use:

struct App {
  #[clap(flatten)]
  config: Config,
}

struct Config {
   ...
}

To make our config values merge correctly, we should take values that parsed via clap as a source:

let mut builder = Config::builder()
    .add_source(Environment::default())
    .add_source(File::new("config/settings", FileFormat::Json))
    .add_source(Config::parse());

In order to make that code works, we need:

  • Implement serde::Serializer for Value, so we can build Value from the config struct itself. (Maybe implementing Into<Value> is enough?)
  • Implement Source/AsyncSourc for Value, so we can use the value as a source (it's trivial and makes sense, right?)

What do you think about this idea?

@Xuanwo Xuanwo changed the title proposal: Implement serde::Serialize and Source/AsyncSource for Value proposal: Implement serde::Serializer and Source/AsyncSource for Value Apr 15, 2022
@matthiasbeyer
Copy link
Collaborator

I am not 100% sure whether this could or even should be done, but I wouldn't mind to see a PR for this.

Please make sure to feature-gate it, so that users of config who don't want to use clap don't have to pull it in! 😄

@Xuanwo
Copy link
Contributor Author

Xuanwo commented Apr 18, 2022

I am not 100% sure whether this could or even should be done, but I wouldn't mind to see a PR for this.

Sadly, I failed to implement this without a huge refactor on Value. 😢

Let's close this issue.

@Xuanwo Xuanwo closed this as completed Apr 18, 2022
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

No branches or pull requests

2 participants