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

Support for configuration files #35

Open
cfcs opened this issue Oct 11, 2015 · 9 comments
Open

Support for configuration files #35

cfcs opened this issue Oct 11, 2015 · 9 comments

Comments

@cfcs
Copy link

cfcs commented Oct 11, 2015

It would be nice to have support for configuration files to supplement the arguments passed on the command line (with the arguments on the command line taking precedence).
The configuration file could itself be a parameter for which the application could specify a default location.

@dbuenzli
Copy link
Owner

Yes it would be nice.

I thought a little bit about this when I did the environment variable support but the right design is not self-evident when you take into account the possibility of being agnostic to the type of configuration file, possibly being able to specify the configuration file on the command line itself or in an environment variable and making good error reports in case of configuration file parse errors.

@dlesbre
Copy link

dlesbre commented Jun 30, 2022

I've had to add this to a large project recently and it was rather a pain... Here are the various hacks I tried and their notable downsides :

  1. Making everything optional : all arguments are optional. I read the command-line once. If a config file is specified with the relevant option, I parse it and update any arguments set to None with the found values. I then check that any required positional arguments are present.

    The problem with this is that required arguments are optional (in brackets [, ]) in the help, which is confusing.

    I tried giving required arguments default value (using t option option and a default of Some None), but it still raises an error when those values are missing (which is somewhat expected, but you could argue that providing a default value to a required argument is weird anyways and could lead to weird behaviors)

  2. (Mis)using the environment variable system. I define a (fake) environment variable for each argument. I get my configuration file from the command line using Cmd.eval_peek_opts, and parse it. Then I parse the full command line with a custom ~env functions that looks into the parsed file for values.

    The problem with that is I get (absent my_fake_variable env) in the documentation (which is isn't true, I don't use environment variables at all)

  3. Evaluating things in the right order. In short, I get my configuration file using Cmd.eval_peek_opts as before, and parse it (ignoring it if I get an Ok ``Help) and then define the rest of my arguments (as optional with a default value if any is found in the configuration file, as required otherwise). This works and produces the expected help output, but it can lead to inconsistent usage message when printing error messages.

I don't think Cmdliner should have its own config file parser as there are a lot of different format that can be used and everyone would have different preferences. (For instance, I use a TOML format where arguments are split into multiple sections, so not just a list of key = value pairs).

What I'd like to have is some sort of lazy default. Either add an option ~from_config:(unit -> t option) when defining arguments or a global function similar to ~env in the evaluation function : ~from_config:(string -> string option) with a defined ~config_key:string in the arguments.

@dbuenzli
Copy link
Owner

The current status isn't ideal but I wouldn't say it's that bad either.

I'm not sure I really understand your objection about Option 1. AFAIR (here) it works since required arguments of your cli should not end up in configuration files.

I think I was rather annoyed by:

  1. The lack of automated doc (e.g. documenting in options which key in the config can set the default, like env var do)
  2. The lack of integration with cmdliner's error reporting.
  3. The inability to precisely know how a value was determined (option, env var or config).

@dlesbre
Copy link

dlesbre commented Jul 1, 2022

Well the problem is just that we do want to be able to specify required arguments in the config file. Its unusual but fairly useful since we can have a few required arguments that are mostly constant (e.g. a path to some other utility/script), which is tedious to specify manually each time.

For the anoying points you mention, I bypassed 1 adding an abstraction layer on top of Cmdliner to add the config information as a prefix in the info ~doc. I don't mind 3 so much but 2 was a bit frustrating yes.

@dbuenzli
Copy link
Owner

dbuenzli commented Jul 1, 2022

Its unusual but fairly useful since we can have a few required arguments that are mostly constant (e.g. a path to some other utility/script), which is tedious to specify manually each time.

But these arguments are required for your program not on the cli. So this

  1. The problem with this is that required arguments are optional (in brackets [, ]) in the help, which is confusing.

is actually neither a problem, nor confusing. It is accurate from the point of view of the cli syntax which is what the help describes and you should not try to circumvent it in my opinion; that would be confusing.

If you have an argument that has no good default and needs to be specified either by cli, env var or config, then mention it in the cmdliner doc string.

Then you should simply bail out with an error later when you get a None after cmdliner cli and config have occurred (either by using Term.term_result or a higher level error mecanism of your tool).

@samoht
Copy link

samoht commented Jul 1, 2023

I was hit by this again in mirage/mirage#1454

It would be nice to have an ?if_absent:(string list -> string option) function threaded to Cmd.eval so we can decide what to do in case an optional argument (with parameters ps) is absent: either evaluating to the default value - like today - if if_absent ps = None or inject another value x if if_absent ps = Some x.

That allows users to define various schemes of the non-present values, for instance, using a config file.

@dbuenzli
Copy link
Owner

dbuenzli commented Jul 1, 2023

As I already mentioned to you there is already support for that with Arg.some' I don't think your proposal brings anything better to the API.

@samoht
Copy link

samoht commented Jul 1, 2023

This doesn't work when your API doesn't have control over whether people are using Arg.some or Arg.some' to build their term. Having if_absent move the decision of using a config file at evaluation time instead of when you build your terms. As for logs and their reporters, you might not know the setup in which these terms are evaluated (e.g. do you provide a config file or read from the CLI or both).

I guess an alternative would be to transform the config file into an argv-like value and somehow combine it with the normal argv. That seems a bit painful, though.

@dbuenzli
Copy link
Owner

dbuenzli commented Jul 1, 2023

This doesn't work when your API doesn't have control over whether people are using Arg.some or Arg.some' to build their term.

Well I'm not sure why this should be the case. Maybe your API is broken then…

Having if_absent move the decision of using a config file at evaluation time instead of when you build your terms.

That's not compositional and it is actually exactly what the current API allows (i.e. solve the problem at evaluation time) without needing to introduce anything else. It just seems that your API is not providing the right hooks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants