diff --git a/packages/shorebird_cli/lib/src/commands/patch/patch_command.dart b/packages/shorebird_cli/lib/src/commands/patch/patch_command.dart index a61267f8..a29f943b 100644 --- a/packages/shorebird_cli/lib/src/commands/patch/patch_command.dart +++ b/packages/shorebird_cli/lib/src/commands/patch/patch_command.dart @@ -47,18 +47,16 @@ class PatchCommand extends ShorebirdCommand { abbr: 'p', help: 'The platform(s) to to build this release for.', allowed: ReleaseType.values.map((e) => e.cliName).toList(), - // TODO(bryanoltman): uncomment this once https://github.com/dart-lang/args/pull/273 lands - // mandatory: true. ) ..addOption( - 'build-number', - help: ''' -An identifier used as an internal version number. -Each build must have a unique identifier to differentiate it from previous builds. -It is used to determine whether one build is more recent than another, with higher numbers indicating more recent build. -On Android it is used as "versionCode". -On Xcode builds it is used as "CFBundleVersion".''', - defaultsTo: '1.0', + CommonArguments.buildNameArg.name, + help: CommonArguments.buildNameArg.description, + defaultsTo: CommonArguments.buildNameArg.defaultValue, + ) + ..addOption( + CommonArguments.buildNumberArg.name, + help: CommonArguments.buildNumberArg.description, + defaultsTo: CommonArguments.buildNumberArg.defaultValue, ) ..addOption( 'target', diff --git a/packages/shorebird_cli/lib/src/commands/patch/patcher.dart b/packages/shorebird_cli/lib/src/commands/patch/patcher.dart index 11c5463c..6ba835f4 100644 --- a/packages/shorebird_cli/lib/src/commands/patch/patcher.dart +++ b/packages/shorebird_cli/lib/src/commands/patch/patcher.dart @@ -6,6 +6,8 @@ import 'package:args/args.dart'; import 'package:mason_logger/mason_logger.dart'; import 'package:path/path.dart' as p; import 'package:shorebird_cli/src/code_push_client_wrapper.dart'; +import 'package:shorebird_cli/src/common_arguments.dart'; +import 'package:shorebird_cli/src/extensions/iterable.dart'; import 'package:shorebird_cli/src/patch_diff_checker.dart'; import 'package:shorebird_cli/src/platform/platform.dart'; import 'package:shorebird_cli/src/release_type.dart'; @@ -139,10 +141,21 @@ https://docs.shorebird.dev/status#link-percentage-ios return []; } - // If the user already provided --build-name or --build-number, we don't - // want to override them. + // If the user provided --build-name or --build-number before the --, we + // don't want to override them. + if (argResults.options.containsAnyOf([ + CommonArguments.buildNameArg.name, + CommonArguments.buildNumberArg.name, + ])) { + return []; + } + + // If the user provided --build-name or --build-number after the --, we + // don't want to override them. if (argResults.rest.any( - (a) => a.startsWith('--build-name') || a.startsWith('--build-number'), + (a) => + a.startsWith('--${CommonArguments.buildNameArg.name}') || + a.startsWith('--${CommonArguments.buildNumberArg.name}'), )) { return []; } diff --git a/packages/shorebird_cli/lib/src/commands/release/release_command.dart b/packages/shorebird_cli/lib/src/commands/release/release_command.dart index 8d1da922..2f7f02a6 100644 --- a/packages/shorebird_cli/lib/src/commands/release/release_command.dart +++ b/packages/shorebird_cli/lib/src/commands/release/release_command.dart @@ -49,14 +49,14 @@ class ReleaseCommand extends ShorebirdCommand { help: 'The product flavor to use when building the app.', ) ..addOption( - 'build-number', - help: ''' -An identifier used as an internal version number. -Each build must have a unique identifier to differentiate it from previous builds. -It is used to determine whether one build is more recent than another, with higher numbers indicating more recent build. -On Android it is used as "versionCode". -On Xcode builds it is used as "CFBundleVersion".''', - defaultsTo: '1.0', + CommonArguments.buildNameArg.name, + help: CommonArguments.buildNameArg.description, + defaultsTo: CommonArguments.buildNameArg.defaultValue, + ) + ..addOption( + CommonArguments.buildNumberArg.name, + help: CommonArguments.buildNumberArg.description, + defaultsTo: CommonArguments.buildNumberArg.defaultValue, ) ..addFlag( 'codesign', diff --git a/packages/shorebird_cli/lib/src/common_arguments.dart b/packages/shorebird_cli/lib/src/common_arguments.dart index e396aa23..abfc50bf 100644 --- a/packages/shorebird_cli/lib/src/common_arguments.dart +++ b/packages/shorebird_cli/lib/src/common_arguments.dart @@ -8,6 +8,7 @@ class ArgumentDescriber { const ArgumentDescriber({ required this.name, required this.description, + this.defaultValue, }); /// Argument name as how the user writes it. @@ -15,11 +16,40 @@ class ArgumentDescriber { /// Argument description that will be shown in the help of the command. final String description; + + /// Default value for this argument. Only provide this if the default value + /// holds across all commands. + final String? defaultValue; } /// A class that houses the name of arguments that are shared between different /// commands and layers. class CommonArguments { + /// The Flutter --build-name argument. + static const buildNameArg = ArgumentDescriber( + name: 'build-name', + description: ''' +An identifier used as an internal version number. +Each build must have a unique identifier to differentiate it from previous builds. +It is used to determine whether one build is more recent than another, with higher numbers indicating more recent build. +On Android it is used as "versionCode". +On Xcode builds it is used as "CFBundleVersion".''', + ); + + /// The Flutter --build-number argument. + static const buildNumberArg = ArgumentDescriber( + name: 'build-number', + description: ''' +A "x.y.z" string used as the version number shown to users. +For each new version of your app, you will provide a version number to differentiate it +from previous versions. +On Android it is used as "versionName". +On Xcode builds it is used as "CFBundleShortVersionString". +On Windows it is used as the major, minor, and patch parts of the product and file +versions.''', + defaultValue: '1.0', + ); + /// A multioption argument that defines constants for the built application. /// These are forwarded to Flutter. static const dartDefineArg = ArgumentDescriber( diff --git a/packages/shorebird_cli/lib/src/extensions/arg_results.dart b/packages/shorebird_cli/lib/src/extensions/arg_results.dart index 7afdbd57..1c4c59c5 100644 --- a/packages/shorebird_cli/lib/src/extensions/arg_results.dart +++ b/packages/shorebird_cli/lib/src/extensions/arg_results.dart @@ -105,6 +105,24 @@ extension ForwardedArgs on ArgResults { bool _isPositionalArgPlatform(String arg) => ReleaseType.values.any((target) => target.cliName == arg); + /// All parsed args with the given name. Because of multioptions, there may + /// be multiple values for a single name, so we return a potentially empty + /// [Iterable] instead of a [String?]. + Iterable _argsNamed(String name) { + if (!wasParsed(name)) { + return []; + } + + final value = this[name]; + if (value is List) { + return value.map((a) => '--$name=$a'); + } else { + return ['--$name=$value']; + } + } + + /// A list of arguments parsed by Shorebird commands that will be forwarded + /// to the underlying Flutter commands (that is, placed after `--`). List get forwardedArgs { final List forwarded; if (rest.isNotEmpty && _isPositionalArgPlatform(rest.first)) { @@ -113,20 +131,14 @@ extension ForwardedArgs on ArgResults { forwarded = rest.toList(); } - if (wasParsed(CommonArguments.dartDefineArg.name)) { - forwarded.addAll( - (this[CommonArguments.dartDefineArg.name] as List).map( - (a) => '--${CommonArguments.dartDefineArg.name}=$a', - ), - ); - } - - if (wasParsed(CommonArguments.dartDefineFromFileArg.name)) { - forwarded.addAll( - (this[CommonArguments.dartDefineFromFileArg.name] as List) - .map((a) => '--${CommonArguments.dartDefineFromFileArg.name}=$a'), - ); - } + forwarded.addAll( + [ + ..._argsNamed(CommonArguments.dartDefineArg.name), + ..._argsNamed(CommonArguments.dartDefineFromFileArg.name), + ..._argsNamed(CommonArguments.buildNameArg.name), + ..._argsNamed(CommonArguments.buildNumberArg.name), + ], + ); return forwarded; } diff --git a/packages/shorebird_cli/lib/src/extensions/iterable.dart b/packages/shorebird_cli/lib/src/extensions/iterable.dart new file mode 100644 index 00000000..09dadbe0 --- /dev/null +++ b/packages/shorebird_cli/lib/src/extensions/iterable.dart @@ -0,0 +1,5 @@ +/// Provides the [containsAnyOf] method to all [Iterable]s. +extension ContainsAnyOf on Iterable { + /// Returns `true` if any of the elements in [elements] are in this iterable. + bool containsAnyOf(Iterable elements) => elements.any(contains); +} diff --git a/packages/shorebird_cli/test/src/commands/patch/android_patcher_test.dart b/packages/shorebird_cli/test/src/commands/patch/android_patcher_test.dart index 40779178..2432d3b7 100644 --- a/packages/shorebird_cli/test/src/commands/patch/android_patcher_test.dart +++ b/packages/shorebird_cli/test/src/commands/patch/android_patcher_test.dart @@ -136,6 +136,7 @@ void main() { shorebirdValidator = MockShorebirdValidator(); shorebirdAndroidArtifacts = MockShorebirdAndroidArtifacts(); + when(() => argResults.options).thenReturn([]); when(() => argResults.rest).thenReturn([]); when(() => argResults.wasParsed(any())).thenReturn(false); diff --git a/packages/shorebird_cli/test/src/commands/patch/ios_patcher_test.dart b/packages/shorebird_cli/test/src/commands/patch/ios_patcher_test.dart index 8148a6df..6ae9142c 100644 --- a/packages/shorebird_cli/test/src/commands/patch/ios_patcher_test.dart +++ b/packages/shorebird_cli/test/src/commands/patch/ios_patcher_test.dart @@ -129,7 +129,7 @@ void main() { shorebirdValidator = MockShorebirdValidator(); xcodeBuild = MockXcodeBuild(); - when(() => argResults['build-number']).thenReturn('1.0'); + when(() => argResults.options).thenReturn([]); when(() => argResults.rest).thenReturn([]); when(() => argResults.wasParsed(any())).thenReturn(false); diff --git a/packages/shorebird_cli/test/src/commands/patch/patcher_test.dart b/packages/shorebird_cli/test/src/commands/patch/patcher_test.dart index 263df22b..9286a98f 100644 --- a/packages/shorebird_cli/test/src/commands/patch/patcher_test.dart +++ b/packages/shorebird_cli/test/src/commands/patch/patcher_test.dart @@ -4,6 +4,7 @@ import 'package:args/args.dart'; import 'package:mocktail/mocktail.dart'; import 'package:shorebird_cli/src/code_push_client_wrapper.dart'; import 'package:shorebird_cli/src/commands/commands.dart'; +import 'package:shorebird_cli/src/common_arguments.dart'; import 'package:shorebird_cli/src/patch_diff_checker.dart'; import 'package:shorebird_cli/src/platform/platform.dart'; import 'package:shorebird_cli/src/release_type.dart'; @@ -45,6 +46,7 @@ void main() { late ArgResults argResults; setUp(() { argResults = MockArgResults(); + when(() => argResults.options).thenReturn([]); }); group('when releaseVersion is not specified', () { @@ -74,12 +76,26 @@ void main() { }); group('when a valid --release-version is specified', () { - group('when --build-name and --build-number are specified', () { + group('when --build-name is specified', () { setUp(() { - when(() => argResults.rest).thenReturn([ - '--build-name=foo', - '--build-number=42', - ]); + when(() => argResults.rest).thenReturn(['--build-name=foo']); + }); + + test('returns an empty list', () { + expect( + _TestPatcher( + argResults: argResults, + flavor: null, + target: null, + ).buildNameAndNumberArgsFromReleaseVersion('1.2.3+4'), + isEmpty, + ); + }); + }); + + group('when --build-number is specified', () { + setUp(() { + when(() => argResults.rest).thenReturn(['--build-number=42']); }); test('returns an empty list', () { @@ -108,6 +124,34 @@ void main() { ); }); }); + + group('when build-name and build-number were parsed as options', () { + setUp(() { + when( + () => argResults.wasParsed(CommonArguments.buildNameArg.name), + ).thenReturn(true); + when( + () => argResults.wasParsed(CommonArguments.buildNumberArg.name), + ).thenReturn(true); + when(() => argResults.options).thenReturn([ + 'release-version', + 'build-name', + 'build-number', + 'platforms', + ]); + }); + + test('returns an empty list', () { + expect( + _TestPatcher( + argResults: argResults, + flavor: null, + target: null, + ).buildNameAndNumberArgsFromReleaseVersion('1.2.3+4'), + isEmpty, + ); + }); + }); }); }); }); diff --git a/packages/shorebird_cli/test/src/extensions/arg_results_test.dart b/packages/shorebird_cli/test/src/extensions/arg_results_test.dart index 9cc20696..6156295d 100644 --- a/packages/shorebird_cli/test/src/extensions/arg_results_test.dart +++ b/packages/shorebird_cli/test/src/extensions/arg_results_test.dart @@ -98,6 +98,14 @@ void main() { CommonArguments.dartDefineFromFileArg.name, help: CommonArguments.dartDefineFromFileArg.description, ) + ..addOption( + CommonArguments.buildNameArg.name, + help: CommonArguments.buildNameArg.description, + ) + ..addOption( + CommonArguments.buildNumberArg.name, + help: CommonArguments.buildNumberArg.description, + ) ..addMultiOption( 'platforms', allowed: ReleaseType.values.map((e) => e.cliName), @@ -178,5 +186,48 @@ void main() { ); }); }); + + group('when build-name and build-number are provided', () { + test('forwards build-name and build-number', () { + final args = [ + '--verbose', + '--', + '--build-name=1.2.3', + '--build-number=4', + ]; + final result = parser.parse(args); + expect(result.forwardedArgs, hasLength(2)); + expect( + result.forwardedArgs, + containsAll( + [ + '--build-name=1.2.3', + '--build-number=4', + ], + ), + ); + }); + }); + + group('when build-name and build-number are before the --', () { + test('forwards build-name and build-number', () { + final args = [ + '--verbose', + '--build-name=1.2.3', + '--build-number=4', + ]; + final result = parser.parse(args); + expect(result.forwardedArgs, hasLength(2)); + expect( + result.forwardedArgs, + containsAll( + [ + '--build-name=1.2.3', + '--build-number=4', + ], + ), + ); + }); + }); }); } diff --git a/packages/shorebird_cli/test/src/extensions/iterable_test.dart b/packages/shorebird_cli/test/src/extensions/iterable_test.dart new file mode 100644 index 00000000..7267abb1 --- /dev/null +++ b/packages/shorebird_cli/test/src/extensions/iterable_test.dart @@ -0,0 +1,18 @@ +import 'package:shorebird_cli/src/extensions/iterable.dart'; +import 'package:test/test.dart'; + +void main() { + group('containsAnyOf', () { + test('returns true when any element is in the iterable', () { + final iterable = [1, 2, 3]; + final elements = [3, 4, 5]; + expect(iterable.containsAnyOf(elements), isTrue); + }); + + test('returns false when no element is in the iterable', () { + final iterable = [1, 2, 3]; + final elements = [4, 5, 6]; + expect(iterable.containsAnyOf(elements), isFalse); + }); + }); +}