[cli_config] First version

[dcharkes]

This is an initial version for package:cli_config.

The only class in the API is the Config class.

/// A hierarchical configuration.
///
/// Configuration can be provided from three sources: commandline arguments,
/// environment variables and configuration files. This configuration makes
/// these accessible via a uniform API.
///
/// Configuration can be provided via the three sources as follows:
/// 1. commandline argument defines as `-Dsome_key=some_value`,
/// 2. environment variables as `SOME_KEY=some_value`, and
/// 3. config files as JSON or YAML as `{'some_key': 'some_value'}`.
///
/// The default lookup behavior is that commandline argument defines take
/// precedence over environment variables, which take precedence over the
/// configuration file.
///
/// If a single value is requested from this configuration, the first source
/// that can provide the value will provide it. For example
/// `config.string('some_key')` with `{'some_key': 'file_value'}` in the
/// config file and `-Dsome_key=cli_value` as commandline argument returns
/// `'cli_value'`. The implication is that you can not remove keys from the
/// configuration file, only overwrite or append them.
///
/// If a list value is requested from this configuration, the values provided
/// by the various sources can be combined or not. For example
/// `config.stringList('some_key', combineAllConfigs: true)` returns
/// `['cli_value', 'file_value']`.
///
/// The config is hierarchical in nature, using `.` as the hierarchy separator
/// for lookup and commandline defines. The hierarchy should be materialized in
/// the JSON or YAML configuration file. For environment variables `__` is used
/// as hierarchy separator.
///
/// Hierarchical configuration can be provided via the three sources as follows:
/// 1. commandline argument defines as `-Dsome_key.some_nested_key=some_value`,
/// 2. environment variables as `SOME_KEY__SOME_NESTED_KEY=some_value`, and
/// 3. config files as JSON or YAML as
///    ```yaml
///    some_key:
///      some_nested_key:
///        some_value
///    ```
///
/// The config is opinionated on the format of the keys in the sources.
/// * Command-line argument keys should be lower-cased alphanumeric
///   characters or underscores, with `.` for hierarchy.
/// * Environment variables keys should be upper-cased alphanumeric
///    characters or underscores, with `__` for hierarchy.
/// * Config files keys should be lower-cased alphanumeric
///   characters or underscores.
///
/// In the API they are made available lower-cased and with underscores, and
/// `.` as hierarchy separator.
class Config { }

This PR is to solicit feedback on multiple levels.

Our format opinions for end users.

What do we accept as valid hierarchical identifers in YAML/JSON/CLI-defines/environment variables?

We prefer underscores in identifiers. (pubspec.yaml and jnigen.yaml do underscores, however ffigen.yaml dashes.) Should we consider something else?

What about hierarchy in environment variables where we cannot use .. Do we use __?

What syntax do we prefer for CLI defines? -Dkey.nested=value (typical define style)? Or --set key.nested=value (Helm style)? Or --key.nested=value (typical CLI style).

Programmable API for package authors

class Config {
  /// Lookup a list of paths in this config.
  ///
  /// If [combineAllConfigs] combines results from cli, environment, and
  /// config file. Otherwise, precedence rules apply.
  ///
  /// If provided, [splitCliPattern] splits cli defines.
  ///
  /// If provided, [splitEnvironmentPattern] splits environment values.
  ///
  /// If [resolveFileUri], resolves the paths in config file relative to the
  /// config file.
  List<Uri>? optionalPathList(
    String key, {
    core.bool combineAllConfigs = true,
    String? splitCliPattern,
    String? splitEnvironmentPattern,
    core.bool resolveFileUri = true,
  });

  // ...
}

A single class with a bunch of getters. The getters define how to treat the values in the various "provider"s. For example whether to split the strings on a character (e.g. splitting PATH on :). The getters also define on whether to combine the values from the various providers or not.

The error flow for package authors

Currently, both creating a Config and failures on config values can throw FormatExceptions. A YAML file could have the wrong format, but also a required config key could be missing which also makes the config the wrong format.

Should we consider having a dedicated type of exception instead?

Implementation considerations.

Please do not yet review those, let's get the other concerns right first.

[coveralls]

Pull Request Test Coverage Report for Build 4542279393

• 235 of 235 (100.0%) changed or added relevant lines in 8 files are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage remained the same at 100.0%

---

Totals
Change from base Build 4505384911:	0.0%
Covered Lines:	235
Relevant Lines:	235

---

💛 - Coveralls

[dcharkes]

@lrhn your thoughts on our APIs are always appreciated!
@jakemac53 @natebosch @devoncarew Any other packages besides JNIgen and FFIgen that we should take a look at while deciding our opinion on the config formats? Do we have any other standards going on in our ecosystem?
cc @mkustermann

[jakemac53]

https://dart.dev/guides/language/effective-dart/design#avoid-starting-a-method-name-with-get

[dcharkes]

Hm, config.string('key') vs config.readString('key') or config.loadString('key'). I guess we don't care about the verb so much.

Unfortunately, config.bool('key') doesn't work, because bool is a keyword.

config.stringValue('key') seems more bloated than using a verb.

get is the shortest verb.

[HosseinYousefi]

I like get because this looks essentially like a map.

An alternative solution is to create a CliResult object that operator [] returns. Then config.getOptionalString('key') becomes config['key'].as<String>(), .as<bool>() and ...

.as<T>() for an unsupported T throws a FormatException.

Edit: Oops, as is also a keyword! maybe .value<String>()?

[HosseinYousefi]

The API here is similar to shared_preferences. And there we have getString and getStringList as well.

[jakemac53]

I would use read, or consider dropping the entire verb and using boolean instead of bool? read is only one more character.

I don't ultimately feel super strongly for this particular case, but I do care that Dart owned/maintained packages follow effective Dart guidelines, so we don't set an example that is contrary to effective Dart. Ultimately people will imitate the types of apis we put out there, and use them as justification for their own, unaware of any deeper reasoning. In fact shared_preferences is a perfect example of this, in this exact thread...

[dcharkes]

https://github.com/flutter/packages/blame/dd8b54101c056190d15e5a1f6ad2b491657f23c2/packages/shared_preferences/lib/shared_preferences.dart I don't seen any code review for our shared_preferences initial PR that introduced this API.

Since read is a verb that communicates not that much here (also, technically speaking, part or the reading has happened already when reading in the various sources), it is a good argument for not having the verb. And being in the case that it should be a getter even though it has is a method with arguments.

The issue here is that the no-verb solution conflicts with keywords. And choosing boolean, integer, and doublee, would create confusion because they are spelled the same as the type they return.

@munificent touched this part of the style guide last, any neat ideas of what to do with conflicts with keywords?

[lrhn]

I'd drop the verb. Changing get to read, which still has no semantic meaning different from "get", "fetch", "lookup" or "find", is not an improvement.

Don't use words that say nothing. Document and name getter-like methods like get getters (noun phrases).
That's what this is.

I'd use string, stringValue, stringValueOf or (more dubious) stringOf.

For bool and int, where we may don't want to get naming conflicts (we can, they are not reserved words, they just shadow the core types), we can use boolean and integer, or boolValue/intValue.

Or just use int and bool and deal with the conflicts:

import "dart:core";
import "dart:core" as core;

....

   core.bool bool(String key) { ... }

(You need to use core.bool everywhere in the class, highly annoying.)

[dcharkes]

Thanks @jakemac53 and @natebosch !

[dcharkes]

Hm, config.string('key') vs config.readString('key') or config.loadString('key'). I guess we don't care about the verb so much.

Unfortunately, config.bool('key') doesn't work, because bool is a keyword.

config.stringValue('key') seems more bloated than using a verb.

get is the shortest verb.

[HosseinYousefi]

LGTM. Thanks @dcharkes!

[lrhn]

Initial comments, didn't get through everything (or much of anything).

[lrhn]

I'd recommend not accepting 0/1.

I'd accept "yes"/"no" before that, but instead of pre-determining which strings you want to accept, I'd parameterize the function below ...

[dcharkes]

The 1 and 0 come from environment variable practises. If we don't want to accept that by default from the other two sources (file/commandline-defines) maybe we should have a map for both env vars and cl defines. (The file source must provide a boolean.)

I like the idea of parameterizing, but I would want to provide a default for people.

[lrhn]

I'd drop the verb. Changing get to read, which still has no semantic meaning different from "get", "fetch", "lookup" or "find", is not an improvement.

Don't use words that say nothing. Document and name getter-like methods like get getters (noun phrases).
That's what this is.

I'd use string, stringValue, stringValueOf or (more dubious) stringOf.

For bool and int, where we may don't want to get naming conflicts (we can, they are not reserved words, they just shadow the core types), we can use boolean and integer, or boolValue/intValue.

Or just use int and bool and deal with the conflicts:

import "dart:core";
import "dart:core" as core;

....

   core.bool bool(String key) { ... }

(You need to use core.bool everywhere in the class, highly annoying.)

[dcharkes]

I've gone with Lasse's approach and just went for bool and made the rest in the file core.bool.

[dcharkes]

It's been quiet here. If there's anymore feedback, please let me know. Otherwise I'll go ahead and merge this.