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: Merge efforts with config-rs #1

Open
mehcode opened this issue Aug 9, 2017 · 6 comments
Open

Proposal: Merge efforts with config-rs #1

mehcode opened this issue Aug 9, 2017 · 6 comments

Comments

@mehcode
Copy link

mehcode commented Aug 9, 2017

Nice crate. I like the idea for a quick method of defaulting some a config struct. However, there seems to be a lot of overlap on functionality in config-rs. Would you like to come join us at config-rs and work on this problem space together?


config-rs is tagged as a configurable configuration library. The idea is for it to support as many possible variants of configuration as possible but to do nothing unless asked.

When merging values on top of defaults, config-rs currently does support JSON, YAML, and TOML; as well as, environment variables. I've plans to support many more formats.

For an example, here is the Usage example in your readme implemented in two different ways.

One possible currently in config-rs:

// Waiting on https://github.com/serde-rs/serde/issues/368
#[derive(Deserialize)]
struct MyConfig {
  width: u32,
  height: u32
}

let mut c = Config::new();

// set_default takes a subset JSONPath as the key 
// so x.y and x[0] will work for setting defaults
c.set_default("width", 640)?;
c.set_default("height", 480)?;

c.merge(config::File::with_name("config.yml"))?;

let config = MyConfig::deserialize(c)?;

And one possible soon in Serde directly you might not have seen:

// Waiting on https://github.com/serde-rs/serde/issues/368
#[derive(Deserialize)]
struct MyConfig {
  #[serde(default_value = "640")]
  width: u32,
  #[serde(default_value = "480")]
  height: u32
}

let config: MyConfig = serde_yaml::from_reader(File::open("config.yml")?)?;

I could help you utilize config internally in this crate if you'd like to keep the macro syntax (it is a bit cleaner for the non-nested struct case). In any case, the more eyes and focus we can get on a common core of configuration utilities, the better.

@kwyse
Copy link
Owner

kwyse commented Aug 10, 2017

Hey @mehcode,

Appreciate you taking the time for this proposal. I agree that there's much overlap.

One of my motivations was to remove the need for stringly-typed key names. Macros allowed for this but introduced other issues like requiring serde as an explicit dependency for clients. But the second snippet you showed that allows defaults to be embedded directly into the struct looks very interesting! I hadn't come across that before and it looks like it would achieve what I intended this crate to be. I'm not sure if this crate would serve much purpose after that feature has been merged, assuming it supports nested structs.

Until then, I'm quite torn on if this crate is more of a default-struct than a default-config. The latter is what I intended but as it stands it's quite generic. However, I think using config-rs would still give a nice feature boost, especially if it can be wrapped in such a way that values can be specified with idents.

I'll give it a whirl. Keen to hear if you have any thoughts on implementation or anything I've mentioned.

@mehcode
Copy link
Author

mehcode commented Aug 10, 2017

@kwyse I had an idea. I'm assuming the motivation for this crate stems from a large usability gap from simple defaults to complex defaults?


For example ..

#[derive(Default)]
struct Foo { bar: i32 }

The above works if you want Foo.bar to have a default of 0. As soon as you don't, you lose the derive and you must manually construct your default.

struct Foo { bar: i32 }

impl Default for Foo {
  fn default() -> Self {
    Foo { bar: 4 }
  }
}

This issue is most apparent with enumerations as #[derive(Default)] cannot work for them.


What about renaming this crate to default2 and rewriting your macro with a proc derive macro? You could make the following API work:

#[derive(Default2)]
#[default = "Red"]
enum Color { Red, Green, Blue }

#[derive(Default2)]
struct Foo {
  #[default = 4]
  bar: u32,

  color: Color
}

The idea is it would generate impl Default.

Furthermore, after this crate is done (as a proof of concept), It'd be interesting to see if something like this could be accepted as an RFC to improve the default Default trait.

I'd use the heck out of something like this.

@kwyse
Copy link
Owner

kwyse commented Aug 12, 2017

@mehcode I really like this idea! I've changed the crate to be a proc macro and implemented a very bare bones version of what you've said in 23da178. Only structs are supported for now but I will implement enums next.

References to configuration have been removed. I'll rename the project shortly. I think it makes sense for this project to remain strictly to override the default values of the Default trait.

@hcpl
Copy link

hcpl commented Aug 14, 2017

@kwyse there is a crate with a similar (but not the same) purpose: https://github.com/nrc/derive-new. You can try to pick some ideas from there.

@kwyse
Copy link
Owner

kwyse commented Aug 16, 2017

Thanks @hcpl. From a quick look it seems like derive-new is applicable under many more use cases than mine, but I'll have a deeper look and use what inspiration I can from it.

@hcpl
Copy link

hcpl commented Aug 16, 2017

@kwyse sure, the functionality between two crates overlaps very much, but there is definitely a use-case for your crate: when something requires a value of type T: Default and you want complex defaults for T.

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

3 participants