feat: allow setting config file path through env var

[tlegrave]

This PRs brings the ability to define cog.toml file path through COG_CONFIG_PATH environment variable (related to #343).

The issue was first proposing to add an argument to the CLI but with the actual architecture this goal would require lots of changes in the code.

If COG_CONFIG_PATH is not specified, the previous value (i.e. cog.toml) will be used.

[cocogitto-bot]

✔️ 81b68d3 - Conventional commits check succeeded.

[oknozor]

Hey @tlegrave, I am not a big fan of doing this via env var while all the other configs are done with the CLI.
We could add a --config flag which wouldn't require much changes.
To make the implementation simple you can define a OnceCell<PathBuf> static variable for COG_CONFIG which would be initialized either using the provided cli arg value or falling back to the default cog.toml path.

[tlegrave]

Hey @oknozor, thanks for taking time to respond.

I went back to the code, with your suggestion in mind. Adding a CLI arg was my first thought, but there is an issue I cannot solve, which is that the lazy SETTINGS is evaluated before the cli parsing.

Adding some print to the main.rs and settings/mod.rs file:

println!("Starting main");    
let cli = Cli::parse();
println!("Cli parsed");

impl TryFrom<&Repository> for Settings {
    type Error = SettingError;

    fn try_from(repo: &Repository) -> Result<Self, Self::Error> {
        println!("Loading settings");
        [...]
    }
}

And the result of the execution:

Starting main
Loading settings
Cli parsed

I was quite surprise at that point. I suppose this is due to the CLI subcommands, that need the SETTINGS variable.
So setting the CONFIG_PATH after the Cli::parse(), as we would like to, doesn't answer the initial need: SETTINGS variable will still point to cog.toml.

Maybe I'm missing something (I'm still quite new to rust !). Hope this helps.

[oknozor]

@tlegrave

I was quite surprise at that point. I suppose this is due to the CLI subcommands, that need the SETTINGS variable.

Exactly, some of the args needs to preload settings in order to dynamically load the args possible values.
For instance:

    InstallHook {
        /// Type of hook to install
        #[arg(value_parser = git_hook_types(),  group = "git-hooks")]
        hook_type: Vec<String>,
        // etc

needs:

fn git_hook_types() -> PossibleValuesParser {
    let hooks = SETTINGS
        .git_hooks
        .keys()
        .map(|hook_type| hook_type.to_string());

    hooks.into()
}

A possible trick here, would be to parse the config flag first without clap, load the config, then pass the arg parsing to `clap.
its not very elegant but at least it would avoid using env vars.
I am not sure about this so I am going to ask on clap repository first, let's see what they think about it.

[tlegrave]

Good idea yes, I was also thinking about some kind of "partial_parse"... Not very conventional, but I feel like this would be the easiest way to break down the cyclic dependency between SETTINGS and the CLI, without changing a lot of logic here!

Waiting for more inputs (the env var at least works for the stuff I had to do, so no need to hurry...).

[oknozor]

Just posted a question on clap discussion space, clap-rs/clap#5232
Let's wait and see what comes back :)

[oknozor]

@tlegrave clap-rs/clap#5232 (comment)