-
Notifications
You must be signed in to change notification settings - Fork 3
feat(config): Support for num Option Type
#95
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -193,11 +193,11 @@ class ComparableValueOption<V extends Comparable> extends ConfigOptionBase<V> { | |
| void validateValue(final V value) { | ||
| super.validateValue(value); | ||
|
|
||
| final mininum = min; | ||
| if (mininum != null && value.compareTo(mininum) < 0) { | ||
| final minimum = min; | ||
| if (minimum != null && value.compareTo(minimum) < 0) { | ||
| throw FormatException( | ||
| '${valueParser.format(value)} is below the minimum ' | ||
| '(${valueParser.format(mininum)})', | ||
| '(${valueParser.format(minimum)})', | ||
| ); | ||
| } | ||
| final maximum = max; | ||
|
|
@@ -246,6 +246,64 @@ class IntOption extends ComparableValueOption<int> { | |
| }) : super(valueParser: const IntParser()); | ||
| } | ||
|
|
||
| /// Converts a source string value to the chosen `num` type `T`. | ||
| /// | ||
| /// Throws a [FormatException] with an appropriate message | ||
| /// if the value cannot be parsed. | ||
| /// | ||
| /// **Note**: `NumParser<Never>.parse` always throws an [UnsupportedError]. | ||
| class NumParser<T extends num> extends ValueParser<T> { | ||
| const NumParser(); | ||
|
|
||
| @override | ||
| T parse(final String value) { | ||
| if (T == double) return double.parse(value) as T; | ||
| if (T == int) return int.parse(value) as T; | ||
| if (T == num) return num.parse(value) as T; | ||
| throw UnsupportedError('NumParser<Never> never parses anything.'); | ||
| } | ||
| } | ||
|
|
||
| /// Number (int/double/num) value configuration option. | ||
| /// | ||
| /// Supports minimum and maximum range checking. | ||
| /// | ||
| /// **Note**: | ||
| /// * Usage of `NumOption<Never>` is unreasonable and not supported | ||
| /// * Reference for usage of `num`: | ||
| /// https://dart.dev/resources/language/number-representation | ||
| class NumOption<T extends num> extends ComparableValueOption<T> { | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Question, should IntOption now extend NumOption? Would there be any drawbacks?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @christerswahn sir, I agree, as I'd have had similar thoughts for the future of A drawback I'm seeing, as a nitpick, is the extremely slight overhead within I'm not sure about it from a design perspective; for example, in extremely rare scenarios where downstream consumer(s) rely on certain reflection-based properties for their use-cases. However, I'd again vote this off for being too pedantic (as almost any change, in general, could then be seen as a breaking change). With respect to a potential Please declare your decision, and I'd adapt the code accordingly:
|
||
| const NumOption({ | ||
| super.argName, | ||
| super.argAliases, | ||
| super.argAbbrev, | ||
| super.argPos, | ||
| super.envName, | ||
| super.configKey, | ||
| super.fromCustom, | ||
| super.fromDefault, | ||
| super.defaultsTo, | ||
| super.helpText, | ||
| super.valueHelp = 'number', | ||
| super.allowedHelp, | ||
| super.group, | ||
| super.allowedValues, | ||
| super.customValidator, | ||
| super.mandatory, | ||
| super.hide, | ||
| super.min, | ||
| super.max, | ||
| }) : super( | ||
| valueParser: (T == double | ||
| ? const NumParser<double>() | ||
| : T == int | ||
| ? const NumParser<int>() | ||
| : T == num | ||
| ? const NumParser<num>() | ||
| : const NumParser<Never>()) as NumParser<T>, | ||
| ); | ||
| } | ||
|
|
||
| /// Parses a date string into a [DateTime] object. | ||
| /// Throws [FormatException] if parsing failed. | ||
| /// | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,161 @@ | ||
| import 'package:config/config.dart' show NumOption, Configuration; | ||
| import 'package:test/test.dart'; | ||
|
|
||
| void main() { | ||
| group('Given a NumOption<double>', () { | ||
| const numOpt = NumOption<double>(argName: 'num', mandatory: true); | ||
| group('when a fractional number is passed', () { | ||
| final config = Configuration.resolveNoExcept( | ||
| options: [numOpt], | ||
| args: ['--num', '123.45'], | ||
| ); | ||
| test('then it is parsed successfully', () { | ||
| expect(config.errors, isEmpty); | ||
| expect( | ||
| config.value(numOpt), | ||
| equals(123.45), | ||
| ); | ||
| }); | ||
| test('then the runtime type is double', () { | ||
| expect( | ||
| config.value(numOpt).runtimeType, | ||
| equals(double), | ||
| ); | ||
| }); | ||
| }); | ||
| group('when an integer number is passed', () { | ||
| final config = Configuration.resolveNoExcept( | ||
| options: [numOpt], | ||
| args: ['--num', '12345'], | ||
| ); | ||
| test('then it is parsed successfully', () { | ||
| expect(config.errors, isEmpty); | ||
| expect( | ||
| config.value(numOpt), | ||
| equals(12345), | ||
| ); | ||
| }); | ||
| test('then the runtime type is double', () { | ||
| expect( | ||
| config.value(numOpt).runtimeType, | ||
| equals(double), | ||
| ); | ||
| }); | ||
| }); | ||
| group('when a non-{double,int} value is passed', () { | ||
| final config = Configuration.resolveNoExcept( | ||
| options: [numOpt], | ||
| args: ['--num', '12i+345j'], | ||
| ); | ||
| test('then it is not parsed successfully', () { | ||
| expect(config.errors, isNotEmpty); | ||
| }); | ||
| }); | ||
| }); | ||
| group('Given a NumOption<int>', () { | ||
| const numOpt = NumOption<int>(argName: 'num', mandatory: true); | ||
| group('when an integer number is passed', () { | ||
| final config = Configuration.resolveNoExcept( | ||
| options: [numOpt], | ||
| args: ['--num', '12345'], | ||
| ); | ||
| test('then it is parsed successfully', () { | ||
| expect(config.errors, isEmpty); | ||
| expect( | ||
| config.value(numOpt), | ||
| equals(12345), | ||
| ); | ||
| }); | ||
| test('then the runtime type is int', () { | ||
| expect( | ||
| config.value(numOpt).runtimeType, | ||
| equals(int), | ||
| ); | ||
| }); | ||
| }); | ||
| group('when a fractional number is passed', () { | ||
| final config = Configuration.resolveNoExcept( | ||
| options: [numOpt], | ||
| args: ['--num', '123.45'], | ||
| ); | ||
| test('then it is not parsed successfully', () { | ||
| expect(config.errors, isNotEmpty); | ||
| }); | ||
| }); | ||
| group('when a non-{double,int} value is passed', () { | ||
| final config = Configuration.resolveNoExcept( | ||
| options: [numOpt], | ||
| args: ['--num', '12i+345j'], | ||
| ); | ||
| test('then it is not parsed successfully', () { | ||
| expect(config.errors, isNotEmpty); | ||
| }); | ||
| }); | ||
| }); | ||
| group('Given a NumOption<num>', () { | ||
| const numOpt = NumOption<num>(argName: 'num', mandatory: true); | ||
| group('when a fractional number is passed', () { | ||
| final config = Configuration.resolveNoExcept( | ||
| options: [numOpt], | ||
| args: ['--num', '123.45'], | ||
| ); | ||
| test('then it is parsed successfully', () { | ||
| expect(config.errors, isEmpty); | ||
| expect( | ||
| config.value(numOpt), | ||
| equals(123.45), | ||
| ); | ||
| }); | ||
| test('then the runtime type is double', () { | ||
| expect( | ||
| config.value(numOpt).runtimeType, | ||
| equals(double), | ||
| ); | ||
| }); | ||
| }); | ||
| group('when an integer number is passed', () { | ||
| final config = Configuration.resolveNoExcept( | ||
| options: [numOpt], | ||
| args: ['--num', '12345'], | ||
| ); | ||
| test('then it is parsed successfully', () { | ||
| expect(config.errors, isEmpty); | ||
| expect( | ||
| config.value(numOpt), | ||
| equals(12345), | ||
| ); | ||
| }); | ||
| test('then the runtime type is int', () { | ||
| expect( | ||
| config.value(numOpt).runtimeType, | ||
| equals(int), | ||
| ); | ||
| }); | ||
| }); | ||
| group('when a non-{double,int} value is passed', () { | ||
| final config = Configuration.resolveNoExcept( | ||
| options: [numOpt], | ||
| args: ['--num', '12i+345j'], | ||
| ); | ||
| test('then it is not parsed successfully', () { | ||
| expect(config.errors, isNotEmpty); | ||
| }); | ||
| }); | ||
| }); | ||
| group('Given a NumOption<Never>', () { | ||
| const numOpt = NumOption<Never>(argName: 'num', mandatory: true); | ||
| group('when any value is passed', () { | ||
| test('then it reports an UnsupportedError', () { | ||
| for (final val in ['123.45', '12345', '12i+345j']) { | ||
| expect( | ||
| () => Configuration.resolveNoExcept( | ||
| options: [numOpt], | ||
| args: ['--num', val], | ||
| ), | ||
| throwsUnsupportedError, | ||
| ); | ||
| } | ||
| }); | ||
| }); | ||
| }); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can this be written as a switch on T instead? If so it will perhaps let you avoid the fallback exception at the end.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@christerswahn sir, I too like the
switchformat more for such use-cases. Unfortunately, Dart does not yet support switching the static value of a generic type parameter.I've rechecked its current status: dart-lang/language#4085
If you wish to have a better alternative here, I'd be happy to know and implement it alongside.