-
Notifications
You must be signed in to change notification settings - Fork 221
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
failed type conversion in .ini file for serde flatten #504
Comments
Hi, thanks for reporting this! A PR would be awesome of course!
What do you mean by this? |
I created my own test code, test it on master branch got error |
The last CI run on master is green though. |
Here is my test code: // tests/file_env_test.rs
use config::{
builder::DefaultState, Config, ConfigBuilder, ConfigError, Environment, File, FileFormat,
};
use serde::Deserialize;
use std::fmt::{Debug, Display};
const ALL_CFG_FILE: &str = "./tests/all.env";
#[derive(Debug, Deserialize)]
struct TestConfig {
#[serde(alias = "TEST__NUMBER")]
pub number: i32,
#[serde(alias = "TEST__STRING")]
pub string: String,
#[serde(alias = "TEST__FLOAT")]
pub float: f32,
#[serde(alias = "TEST__BOOLEAN")]
pub boolean: bool,
}
#[test]
fn test_file_config_success() {
let cfg = Config::builder()
.add_source(config::File::new(ALL_CFG_FILE, FileFormat::Ini))
.build()
.unwrap()
.try_deserialize::<TestConfig>();
dbg!(&cfg);
assert!(cfg.is_ok());
} ini file
My original issue is about type incompatibility on nested configs parsing from .ini file. |
There're many commits unmerged from v0.13.4 into master, hence different version. |
This is the my commit id: e30debf |
For reference the INI support is being refactored a little here: #470 That doesn't change the logic you draw attention to though. The reason we are calling The other formats often parse the strings themselves and provide a
Yes, that sort of improvement could probably be added to (or after) the referenced refactor PR. It is intended for It can be a bit problematic depending on the value I think 🤷♂️
Here a string to a float: and likewise string (or whatever other intermediary value in our container) to a bool type of the user config struct: I think the issue is more related to that area, than |
Okay, then I'll just need to use that version after release. |
I believe there are other issues related to serde deserializing (the internal
I don't know if your concern will be fixed. Addressing INI to parse from The value should already be captured into Some formats are also known to be lossy in conversion into the |
For now, I'll just refer to my commit in my fork. After this is fixed/refactored as a whole, will switch back. |
I think other formats are all fine, but for .ini/.env, when I flatten the structs into another struct, type conversion failed. I checked the code and found this:
https://github.com/mehcode/config-rs/blob/b3bda2c37ea4823b6842ce0f08fd281c07374d60/src/file/format/ini.rs#L21
https://github.com/mehcode/config-rs/blob/b3bda2c37ea4823b6842ce0f08fd281c07374d60/src/file/format/ini.rs#L30
All code are string-ed rightaway.
Should it be parsed like this?: https://github.com/mehcode/config-rs/blob/b3bda2c37ea4823b6842ce0f08fd281c07374d60/src/env.rs#L282
I test locally and it works and was about to create PR, but confuse with master branch status I faced unexpected error, unlike in v0.13.4.
The text was updated successfully, but these errors were encountered: