Restructure config

[JMicheli]

As currently written, Stump's global state is stored in environment variables. For example, in thumbnail.rs, generate_thumbnail() gets thumbnail_path from config::get_thumbnails_dir(), which is defined as follows:

/// Gets the Stump config directory. If the directory does not exist, it will be created. If
/// the path is not a directory (only possible if overridden using STUMP_CONFIG_DIR) it will
/// panic.
pub fn get_config_dir() -> PathBuf {
    let config_dir = std::env::var("STUMP_CONFIG_DIR")
        .map(|val| {
            if val.is_empty() {
                home().join(".stump")
            } else {
                PathBuf::from(val)
            }
        })
        .unwrap_or_else(|_| home().join(".stump"));

    check_configuration_dir(&config_dir);

    config_dir
}

// [omitting unrelated code]

pub fn get_thumbnails_dir() -> PathBuf {
    let thumbnails_dir = get_config_dir().join("thumbnails");

    check_configuration_dir(&thumbnails_dir);

    thumbnails_dir
}

Basically, any time a configuration variable is needed, an std::env::var call is used to extract the variable from the environment and then that value is used. Because of this, Stump must read configuration variables from the config TOML file at startup and then write them into the environment to ensure that they will be accessible when such calls are made.

This strikes me as suboptimal because it makes most functions nondeterministic based on their inputs. If the environment variables were to be changed (for whatever reason) while Stump is running then they would behave differently, even with the same input variables. I think that, ultimately, this is an antipattern, and it is one that will become increasingly difficult to undo as development continues.

My proposal is to rewrite the configuration of Stump so that it behaves the same way that it does now (i.e., loaded from the TOML file and then from the environment at startup) but instead stores the config in a struct which the program holds onto throughout its runtime and passes to the appropriate functions when they are called. Given that this is not a difficult task, just a laborious one, I'd of course be willing to work on it myself, but I want to open the floor to discussion before starting. Any and all thoughts (including reasons that this may not be as good an idea as I think) would be appreciated.

[aaronleopold]

Hey 👋 thanks for writing this up! Sorry I'm only just now getting to it:

This strikes me as suboptimal because it makes most functions nondeterministic based on their inputs. If the environment variables were to be changed (for whatever reason) while Stump is running then they would behave differently, even with the same input variables. I think that, ultimately, this is an antipattern, and it is one that will become increasingly difficult to undo as development continues.

I think this is the strongest argument for your proposed changes 👍 I don't think I have any objections, dealing with a proper config struct will be more maintainable in the long run IMO. One thought is that clap might be able to be used to aid in this refactor, where you could try and make the config clap-compatible (see the cli crate in this monorepo) and default to environment variables.