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

Using var_os instead of var? #26

Open
Nokel81 opened this issue Jan 14, 2020 · 4 comments
Open

Using var_os instead of var? #26

Nokel81 opened this issue Jan 14, 2020 · 4 comments

Comments

@Nokel81
Copy link

Nokel81 commented Jan 14, 2020

I have noticed that this crate uses env::var instead of the env::var_os method. For some types, such as PathBuf there is a need to start with OsStrBuf, and given that most of this library uses the Into<T> (or TryInto<T>) it might be possible to do this change without exposing it to the outside world.

@greyblake
Copy link
Owner

Hi. Thanks for the report.

I do not mind adding this change.
But you could you please provide ann example to illustrate how the difference between env::var and env::var_os matters? Or which use cases env::var are not possible, which are possible with env::var_os.

I've tried this one with PathBuf:

#[derive(Envconfig)]
#[derive(Debug)]
pub struct TheConfig {
    #[envconfig(from = "DIR_PATH")]
    pub dir_path: ::std::path::PathBuf
}

I works as I expected, but I guess I miss some details.

@Nokel81
Copy link
Author

Nokel81 commented Jan 15, 2020

It probably would work, except for when the environment variable doesn't contain valid UTF-8 (like on Windows where the OS strings are UTF-16). Unfortunately there doesn't seem to be a way to convert these to/from String. The env::var command checks to see if the string is valid UTF-8 but does not try and convert it.

@Nokel81 Nokel81 changed the title Using var_os instead of vs? Using var_os instead of var? Feb 5, 2020
@corhere
Copy link

corhere commented Feb 5, 2020

But you could you please provide ann example to illustrate how the difference between env::var and env::var_os matters?

I have prepared an example on the Rust playground which demonstrates how using env::var in place of env::var_os can make it impossible to open a file from a path stored in an environment variable if the path cannot be converted to Unicode.

https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=e3eb62f2920f8da1d63156f80f06f5dd

@greyblake
Copy link
Owner

@corhere Thanks for the example.

@Nokel81 I was not able to figure out how to do it gracefully yet.. If you have any ideas, they're welcome :)

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