envconfig-rs icon indicating copy to clipboard operation
envconfig-rs copied to clipboard

Using var_os instead of var?

Open Nokel81 opened this issue 4 years ago • 4 comments

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.

Nokel81 avatar Jan 14 '20 19:01 Nokel81

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.

greyblake avatar Jan 15 '20 09:01 greyblake

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 avatar Jan 15 '20 13:01 Nokel81

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

corhere avatar Feb 05 '20 17:02 corhere

@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 :)

greyblake avatar Feb 20 '20 20:02 greyblake