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

Config does not support capital letter #26

Closed
WLBF opened this issue Mar 27, 2017 · 5 comments
Closed

Config does not support capital letter #26

WLBF opened this issue Mar 27, 2017 · 5 comments
Labels

Comments

@WLBF
Copy link

WLBF commented Mar 27, 2017

I am using a json file as configuration file, however I can't get correct value with c.get("Server") when there are capital letters in the key field, then I found this:

    pub fn get(&self, key_path: &str) -> Option<Value> {
        let key_expr: path::Expression = match key_path.to_lowercase().parse() {
            Ok(expr) => expr,
            Err(_) => {
                // TODO: Log warning here
                return None;
            }
        };

        self.path_get(key_expr).cloned()
    }

I am curious why it design like this, am I missing something.

@mehcode mehcode added the C-bug label Mar 27, 2017
@mehcode
Copy link
Collaborator

mehcode commented Mar 27, 2017

The idea is configuration keys should not be case-sensitive. Eg get("server"), get("SeRver") should both map to the same key.

I don't have any tests for that case and its likely I'm not ensuring keys get case folded before they get stored (which would make it impossible to get them out).

I'll make sure this works in the upcoming version.

@mehcode
Copy link
Collaborator

mehcode commented Jun 14, 2017

Fixed on master

@mehcode mehcode closed this as completed Jun 14, 2017
@kdar
Copy link

kdar commented Jun 29, 2017

I'm running into this issue with TOML. Per the TOML spec, TOML is case sensitive.
https://github.com/toml-lang/toml#spec
Does not make sense to force lowercase.

@mehcode
Copy link
Collaborator

mehcode commented Jun 29, 2017

@kdar What version of Config are you using? Latest?

https://github.com/mehcode/config-rs/blob/master/src/file/format/toml.rs#L29

That should be definitely be working.


I'm running into this issue with TOML. Per the TOML spec, TOML is case sensitive.

I understand. But this library is not just a TOML parser. There is a great library if you just want to do that: toml-rs. In order to be able to merge configuration properties from N sources we must be case-insensitive. Consider an environment like APP_REDIS_PASSWORD=password. We need that to overwrite configuration from a toml file such as redis = { password = "b" }.

For similar reasons, the configuration properties are not strongly typed. The source may only allow strings (like the environment) but we want to be able to get a boolean or a Date from it.

@kdar
Copy link

kdar commented Jun 29, 2017

Alright, I'll use toml-rs. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants