Skip to content

Commit

Permalink
[cli_config] Make string list and path list consistent (#59)
Browse files Browse the repository at this point in the history
  • Loading branch information
dcharkes authored Mar 31, 2023
1 parent 56dc83b commit ce5025f
Show file tree
Hide file tree
Showing 6 changed files with 130 additions and 22 deletions.
2 changes: 1 addition & 1 deletion pkgs/cli_config/lib/src/cli_source.dart
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ class CliSource extends Source {
}

@override
List<String>? stringList(
List<String>? optionalStringList(
String key, {
String? splitPattern,
}) {
Expand Down
81 changes: 74 additions & 7 deletions pkgs/cli_config/lib/src/config.dart
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ import 'source.dart';
///
/// 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
/// `config.optionalStringList('some_key', combineAllConfigs: true)` returns
/// `['cli_value', 'file_value']`.
///
/// The config is hierarchical in nature, using `.` as the hierarchy separator
Expand Down Expand Up @@ -214,7 +214,7 @@ class Config {
return value!;
}

/// Lookup a nullable string value in this config.
/// Lookup an optional string value in this config.
///
/// First tries CLI argument defines, then environment variables, and
/// finally the config file.
Expand All @@ -231,7 +231,40 @@ class Config {
return value;
}

/// Lookup a nullable string list in this config.
/// Lookup an optional string list in this config.
///
/// If none of the sources provide a list, lookup will fail.
/// If an empty list is provided by one of the sources, lookup wil succeed.
///
/// First tries CLI argument defines, then environment variables, and
/// finally the config file.
///
/// If [combineAllConfigs] combines results from cli, environment, and
/// config file. Otherwise, precedence rules apply.
///
/// If provided, [splitCliPattern] splits cli defines.
/// For example: `-Dfoo=bar;baz` can be split on `;`.
/// If not provided, a list can still be provided with multiple cli defines.
/// For example: `-Dfoo=bar -Dfoo=baz`.
///
/// If provided, [splitEnvironmentPattern] splits environment values.
List<String> stringList(
String key, {
core.bool combineAllConfigs = true,
String? splitCliPattern,
String? splitEnvironmentPattern,
}) {
final value = optionalStringList(
key,
combineAllConfigs: combineAllConfigs,
splitCliPattern: splitCliPattern,
splitEnvironmentPattern: splitEnvironmentPattern,
);
_throwIfNull(key, value);
return value!;
}

/// Lookup an optional string list in this config.
///
/// First tries CLI argument defines, then environment variables, and
/// finally the config file.
Expand All @@ -245,7 +278,7 @@ class Config {
/// For example: `-Dfoo=bar -Dfoo=baz`.
///
/// If provided, [splitEnvironmentPattern] splits environment values.
List<String>? stringList(
List<String>? optionalStringList(
String key, {
core.bool combineAllConfigs = true,
String? splitCliPattern,
Expand All @@ -259,7 +292,7 @@ class Config {
}.entries) {
final source = entry.key;
final splitPattern = entry.value;
final value = source.stringList(key, splitPattern: splitPattern);
final value = source.optionalStringList(key, splitPattern: splitPattern);
if (value != null) {
if (combineAllConfigs) {
(result ??= []).addAll(value);
Expand Down Expand Up @@ -452,6 +485,40 @@ class Config {

/// Lookup a list of paths in this config.
///
/// If none of the sources provide a path, lookup will fail.
/// If an empty list is provided by one of the sources, lookup wil succeed.
///
/// 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 [resolveUri], resolves the paths in a source relative to the base
/// uri of that source. The base uri for the config file is the path of the
/// file. The base uri for environment values is the current working
/// directory.
List<Uri> pathList(
String key, {
core.bool combineAllConfigs = true,
String? splitCliPattern,
String? splitEnvironmentPattern,
core.bool resolveUri = true,
}) {
final value = optionalPathList(
key,
combineAllConfigs: combineAllConfigs,
splitCliPattern: splitCliPattern,
splitEnvironmentPattern: splitEnvironmentPattern,
resolveUri: resolveUri,
);
_throwIfNull(key, value);
return value!;
}

/// Lookup an optional list of paths in this config.
///
/// If [combineAllConfigs] combines results from cli, environment, and
/// config file. Otherwise, precedence rules apply.
///
Expand All @@ -478,7 +545,7 @@ class Config {
}.entries) {
final source = entry.key;
final splitPattern = entry.value;
final paths = source.stringList(
final paths = source.optionalStringList(
key,
splitPattern: splitPattern,
);
Expand All @@ -504,7 +571,7 @@ class Config {
/// Lookup a value of type [T] in this configuration.
///
/// Does not support specialized options such as `splitPattern`. One must
/// use the specialized methods such as [stringList] for that.
/// use the specialized methods such as [optionalStringList] for that.
///
/// If sources cannot lookup type [T], they return null.
T valueOf<T>(String key) {
Expand Down
2 changes: 1 addition & 1 deletion pkgs/cli_config/lib/src/environment_source.dart
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ class EnvironmentSource extends Source {
String? optionalString(String key) => _environment[key];

@override
List<String>? stringList(
List<String>? optionalStringList(
String key, {
String? splitPattern,
}) {
Expand Down
2 changes: 1 addition & 1 deletion pkgs/cli_config/lib/src/file_source.dart
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ class FileSource extends Source {
String? optionalString(String key) => optionalValueOf<String>(key);

@override
List<String>? stringList(
List<String>? optionalStringList(
String key, {
String? splitPattern,
}) {
Expand Down
6 changes: 3 additions & 3 deletions pkgs/cli_config/lib/src/source.dart
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ abstract class Source {
/// Lookup a nullable string list.
///
/// If provided, [splitPattern] splits config.
List<String>? stringList(
List<String>? optionalStringList(
String key, {
String? splitPattern,
});
Expand All @@ -31,7 +31,7 @@ abstract class Source {
/// Lookup an optional value of type [T].
///
/// Does not support specialized options such as `splitPattern`. One must
/// use the specialized methods such as [stringList] for that.
/// use the specialized methods such as [optionalStringList] for that.
///
/// Returns `null` if the source cannot provide a value of type [T].
T? optionalValueOf<T>(String key) {
Expand All @@ -42,7 +42,7 @@ abstract class Source {
return optionalString(key) as T?;
}
if (T == List<String>) {
return stringList(key) as T?;
return optionalStringList(key) as T?;
}
return null;
}
Expand Down
59 changes: 50 additions & 9 deletions pkgs/cli_config/test/cli_config_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import 'package:test/test.dart';
import 'helpers.dart';

void main() {
test('stringList', () {
test('optionalStringList', () {
const path1 = 'path/in/cli_arguments/';
const path2 = 'path/in/cli_arguments_2/';
const path3 = 'path/in/environment/';
Expand Down Expand Up @@ -39,7 +39,7 @@ void main() {
);

{
final result = config.stringList(
final result = config.optionalStringList(
'build.out_dir',
combineAllConfigs: true,
splitEnvironmentPattern: ':',
Expand All @@ -48,7 +48,7 @@ void main() {
}

{
final result = config.stringList(
final result = config.optionalStringList(
'build.out_dir',
combineAllConfigs: false,
splitEnvironmentPattern: ':',
Expand Down Expand Up @@ -264,9 +264,9 @@ void main() {
);
});

test('CLI split stringlist', () {
test('CLI split optionalStringList', () {
final config = Config(commandLineDefines: ['key=value;value2']);
final value = config.stringList('key', splitCliPattern: ';');
final value = config.optionalStringList('key', splitCliPattern: ';');
expect(value, ['value', 'value2']);
});

Expand Down Expand Up @@ -332,15 +332,16 @@ void main() {
);
});

test('environment split stringlist', () {
test('environment split optionalStringList', () {
final config = Config(environment: {'key': 'value;value2'});
final value = config.stringList('key', splitEnvironmentPattern: ';');
final value =
config.optionalStringList('key', splitEnvironmentPattern: ';');
expect(value, ['value', 'value2']);
});

test('environment non split stringlist', () {
test('environment non split optionalStringList', () {
final config = Config(environment: {'key': 'value'});
final value = config.stringList('key');
final value = config.optionalStringList('key');
expect(value, ['value']);
});

Expand Down Expand Up @@ -458,4 +459,44 @@ void main() {
expect(() => config.optionalDouble('not_parsable'), throwsFormatException);
expect(() => config.optionalDouble('not_parsable2'), throwsFormatException);
});

test('stringList and optionalStringList', () {
{
final config = Config(
fileParsed: {},
);

expect(config.optionalStringList('my_list'), null);
expect(() => config.stringList('my_list'), throwsFormatException);
}

{
final config = Config(
fileParsed: {'my_list': <String>[]},
);

expect(config.optionalStringList('my_list'), <String>[]);
expect(config.stringList('my_list'), <String>[]);
}
});

test('pathList and optionalPathList', () {
{
final config = Config(
fileParsed: {},
);

expect(config.optionalPathList('my_list'), null);
expect(() => config.pathList('my_list'), throwsFormatException);
}

{
final config = Config(
fileParsed: {'my_list': <String>[]},
);

expect(config.optionalPathList('my_list'), <String>[]);
expect(config.pathList('my_list'), <String>[]);
}
});
}

0 comments on commit ce5025f

Please sign in to comment.