Manually create a xdg::BaseDirectories value

[piegamesde]

I'd like to mock some data for testing. I tried modifying environment variables at first but this failed because other parts of the application do rely on the real paths still being present

[whitequark]

This seems useful. I'm happy to merge a PR adding a new constructor creating a mock BaseDirectories value. (Something like xdg::BaseDirectories::with_mock_directories(...)?) Alternatively we could just make the fields public. I'm not sure if anything requires them to be private but on a quick glance it doesn't seem like anything does?

[julianandrews]

I started having a go at writing this and I'm having trouble coming up with an interface that isn't kind of ugly for the with_mock_directories() approach - it would need nine positional arguments, many of the same type, or some sort of opaque lambda, neither of which is a great public interface.

I think probably the best approach is either to make the fields public, or use some sort of builder pattern.

The builder pattern would probably look something like:

pub struct BaseDirectoriesBuilder { ... }

impl BaseDirectoriesBuilder {
        pub fn new() -> BaseDirectoriesBuilder { ... }

        pub fn with_prefix<P: AsRef<Path>>(prefix: P) -> BaseDirectoriesBuilder { ... }

        pub fn with_profile<P1, P2>(prefix: P1, profile: P2 -> BaseDirectoriesBuilder
        where
            P1: AsRef<Path>,
            P2: AsRef<Path>,
        { ... }

        pub fn with_data_home<P: AsRef<Path>>(self, data_home: P) -> BaseDirectoriesBuilder { ... }

        ...

        pub fn build(self) -> BaseDirectories {}
}

I'm leaning towards just making the fields public, and possibly adding a convenience method to build an "clean" instance. Something like:

impl BaseDirectories {
    pub fn empty() -> BaseDirectories {
        BaseDirectories {
            PathBuf::new(),
            PathBuf::new(),
            None,
            None,
            None,
            Vec::new(),
            Vec::new(),
            None,
    }
}

@whitequark - thoughts?

[piegamesde]

Looking at the current code, there already exists the with_env function, and it is also used for testing. So my proposal would be to extend all existing constructors with a _env variant where you can inject custom environment variables. Ideally, one could call them with std::env::var_os as function and get the same thing as the original constructors.

This would also allow thing like |var| match var { "XDG_DATA_HOME" => Some("/tmp"), _ => std::env::var_os(var) }, which I find rather elegant.

[whitequark]

Is there any downside at all to making the fields public?

[whitequark]

o my proposal would be to extend all existing constructors with a _env variant where you can inject custom environment variables. Ideally, one could call them with std::env::var_os as function and get the same thing as the original constructors.

But this looks sensible to me as well.

[piegamesde]

About making the fields public, I'm not opposed to that it's just I don't know how the "prefix" fields interact with the other ones. Are there any invariants of any kind that should be upheld?

[whitequark]

I don't quite remember, I wrote this code many many years ago originally...