Skip to content
This repository was archived by the owner on Feb 22, 2023. It is now read-only.

[flutter_plugin_tool] Fix iOS/macOS naming #4861

Merged
merged 1 commit into from
Feb 16, 2022
Merged

[flutter_plugin_tool] Fix iOS/macOS naming #4861

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 22 additions & 22 deletions script/tool/lib/src/build_examples_command.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ const int _exitInvalidPluginToolsConfig = 4;

// Flutter build types. These are the values passed to `flutter build <foo>`.
const String _flutterBuildTypeAndroid = 'apk';
const String _flutterBuildTypeIos = 'ios';
const String _flutterBuildTypeIOS = 'ios';
const String _flutterBuildTypeLinux = 'linux';
const String _flutterBuildTypeMacOS = 'macos';
const String _flutterBuildTypeWeb = 'web';
Expand All @@ -48,12 +48,12 @@ class BuildExamplesCommand extends PackageLoopingCommand {
ProcessRunner processRunner = const ProcessRunner(),
Platform platform = const LocalPlatform(),
}) : super(packagesDir, processRunner: processRunner, platform: platform) {
argParser.addFlag(kPlatformLinux);
argParser.addFlag(kPlatformMacos);
argParser.addFlag(kPlatformWeb);
argParser.addFlag(kPlatformWindows);
argParser.addFlag(kPlatformWinUwp);
argParser.addFlag(kPlatformIos);
argParser.addFlag(platformLinux);
argParser.addFlag(platformMacOS);
argParser.addFlag(platformWeb);
argParser.addFlag(platformWindows);
argParser.addFlag(platformWinUwp);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't you want platformWinUWP?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

UWP is three letters, not two.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"iOS" is 3 letters too. It's not a 2 letter acronym. If this is what you all decided upon that's fine but I think we need to do a better job describing the algorithm like the Java style guide does. I think the way that is written leaves zero ambiguities.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

UWP is unambiguously not a two-letter acronym (which is what the Dart style guide makes an exception for). platfromWinUWP is therefore unambiguously against Dart style.

argParser.addFlag(platformIOS);
argParser.addFlag(_platformFlagApk);
argParser.addOption(
kEnableExperiment,
Expand All @@ -68,39 +68,39 @@ class BuildExamplesCommand extends PackageLoopingCommand {
<String, _PlatformDetails>{
_platformFlagApk: const _PlatformDetails(
'Android',
pluginPlatform: kPlatformAndroid,
pluginPlatform: platformAndroid,
flutterBuildType: _flutterBuildTypeAndroid,
),
kPlatformIos: const _PlatformDetails(
platformIOS: const _PlatformDetails(
'iOS',
pluginPlatform: kPlatformIos,
flutterBuildType: _flutterBuildTypeIos,
pluginPlatform: platformIOS,
flutterBuildType: _flutterBuildTypeIOS,
extraBuildFlags: <String>['--no-codesign'],
),
kPlatformLinux: const _PlatformDetails(
platformLinux: const _PlatformDetails(
'Linux',
pluginPlatform: kPlatformLinux,
pluginPlatform: platformLinux,
flutterBuildType: _flutterBuildTypeLinux,
),
kPlatformMacos: const _PlatformDetails(
platformMacOS: const _PlatformDetails(
'macOS',
pluginPlatform: kPlatformMacos,
pluginPlatform: platformMacOS,
flutterBuildType: _flutterBuildTypeMacOS,
),
kPlatformWeb: const _PlatformDetails(
platformWeb: const _PlatformDetails(
'web',
pluginPlatform: kPlatformWeb,
pluginPlatform: platformWeb,
flutterBuildType: _flutterBuildTypeWeb,
),
kPlatformWindows: const _PlatformDetails(
platformWindows: const _PlatformDetails(
'Win32',
pluginPlatform: kPlatformWindows,
pluginPlatform: platformWindows,
pluginPlatformVariant: platformVariantWin32,
flutterBuildType: _flutterBuildTypeWin32,
),
kPlatformWinUwp: const _PlatformDetails(
platformWinUwp: const _PlatformDetails(
'UWP',
pluginPlatform: kPlatformWindows,
pluginPlatform: platformWindows,
pluginPlatformVariant: platformVariantWinUwp,
flutterBuildType: _flutterBuildTypeWinUwp,
),
Expand Down Expand Up @@ -288,7 +288,7 @@ class BuildExamplesCommand extends PackageLoopingCommand {
if (!uwpDirectory.existsSync()) {
print('Creating temporary winuwp folder');
final int exitCode = await processRunner.runAndStream(flutterCommand,
<String>['create', '--platforms=$kPlatformWinUwp', '.'],
<String>['create', '--platforms=$platformWinUwp', '.'],
workingDir: example.directory);
if (exitCode == 0) {
temporaryPlatformDirectory = uwpDirectory;
Expand Down
16 changes: 8 additions & 8 deletions script/tool/lib/src/common/core.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,40 +11,40 @@ import 'package:yaml/yaml.dart';
typedef Print = void Function(Object? object);

/// Key for APK (Android) platform.
const String kPlatformAndroid = 'android';
const String platformAndroid = 'android';

/// Key for IPA (iOS) platform.
const String kPlatformIos = 'ios';
const String platformIOS = 'ios';

/// Key for linux platform.
const String kPlatformLinux = 'linux';
const String platformLinux = 'linux';

/// Key for macos platform.
const String kPlatformMacos = 'macos';
const String platformMacOS = 'macos';

/// Key for Web platform.
const String kPlatformWeb = 'web';
const String platformWeb = 'web';

/// Key for windows platform.
///
/// Note that this corresponds to the Win32 variant for flutter commands like
/// `build` and `run`, but is a general platform containing all Windows
/// variants for purposes of the `platform` section of a plugin pubspec).
const String kPlatformWindows = 'windows';
const String platformWindows = 'windows';

/// Key for WinUWP platform.
///
/// Note that UWP is a platform for the purposes of flutter commands like
/// `build` and `run`, but a variant of the `windows` platform for the purposes
/// of plugin pubspecs).
const String kPlatformWinUwp = 'winuwp';
const String platformWinUwp = 'winuwp';

/// Key for Win32 variant of the Windows platform.
const String platformVariantWin32 = 'win32';

/// Key for UWP variant of the Windows platform.
///
/// See the note on [kPlatformWinUwp].
/// See the note on [platformWinUwp].
const String platformVariantWinUwp = 'uwp';

/// Key for enable experiment.
Expand Down
16 changes: 8 additions & 8 deletions script/tool/lib/src/common/plugin_utils.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,12 +39,12 @@ bool pluginSupportsPlatform(
PlatformSupport? requiredMode,
String? variant,
}) {
assert(platform == kPlatformIos ||
platform == kPlatformAndroid ||
platform == kPlatformWeb ||
platform == kPlatformMacos ||
platform == kPlatformWindows ||
platform == kPlatformLinux);
assert(platform == platformIOS ||
platform == platformAndroid ||
platform == platformWeb ||
platform == platformMacOS ||
platform == platformWindows ||
platform == platformLinux);

final YamlMap? platformEntry =
_readPlatformPubspecSectionForPlugin(platform, plugin);
Expand Down Expand Up @@ -73,7 +73,7 @@ bool pluginSupportsPlatform(
// Platforms with variants have a default variant when unspecified for
// backward compatibility. Must match the flutter tool logic.
const Map<String, String> defaultVariants = <String, String>{
kPlatformWindows: platformVariantWin32,
platformWindows: platformVariantWin32,
};
if (variant != defaultVariants[platform]) {
return false;
Expand All @@ -87,7 +87,7 @@ bool pluginSupportsPlatform(
/// Returns true if [plugin] includes native code for [platform], as opposed to
/// being implemented entirely in Dart.
bool pluginHasNativeCodeForPlatform(String platform, RepositoryPackage plugin) {
if (platform == kPlatformWeb) {
if (platform == platformWeb) {
// Web plugins are always Dart-only.
return false;
}
Expand Down
63 changes: 31 additions & 32 deletions script/tool/lib/src/drive_examples_command.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,19 +25,19 @@ class DriveExamplesCommand extends PackageLoopingCommand {
ProcessRunner processRunner = const ProcessRunner(),
Platform platform = const LocalPlatform(),
}) : super(packagesDir, processRunner: processRunner, platform: platform) {
argParser.addFlag(kPlatformAndroid,
argParser.addFlag(platformAndroid,
help: 'Runs the Android implementation of the examples');
argParser.addFlag(kPlatformIos,
argParser.addFlag(platformIOS,
help: 'Runs the iOS implementation of the examples');
argParser.addFlag(kPlatformLinux,
argParser.addFlag(platformLinux,
help: 'Runs the Linux implementation of the examples');
argParser.addFlag(kPlatformMacos,
argParser.addFlag(platformMacOS,
help: 'Runs the macOS implementation of the examples');
argParser.addFlag(kPlatformWeb,
argParser.addFlag(platformWeb,
help: 'Runs the web implementation of the examples');
argParser.addFlag(kPlatformWindows,
argParser.addFlag(platformWindows,
help: 'Runs the Windows (Win32) implementation of the examples');
argParser.addFlag(kPlatformWinUwp,
argParser.addFlag(platformWinUwp,
help:
'Runs the UWP implementation of the examples [currently a no-op]');
argParser.addOption(
Expand All @@ -64,13 +64,13 @@ class DriveExamplesCommand extends PackageLoopingCommand {
@override
Future<void> initializeRun() async {
final List<String> platformSwitches = <String>[
kPlatformAndroid,
kPlatformIos,
kPlatformLinux,
kPlatformMacos,
kPlatformWeb,
kPlatformWindows,
kPlatformWinUwp,
platformAndroid,
platformIOS,
platformLinux,
platformMacOS,
platformWeb,
platformWindows,
platformWinUwp,
];
final int platformCount = platformSwitches
.where((String platform) => getBoolArg(platform))
Expand All @@ -85,12 +85,12 @@ class DriveExamplesCommand extends PackageLoopingCommand {
throw ToolExit(_exitNoPlatformFlags);
}

if (getBoolArg(kPlatformWinUwp)) {
if (getBoolArg(platformWinUwp)) {
logWarning('Driving UWP applications is not yet supported');
}

String? androidDevice;
if (getBoolArg(kPlatformAndroid)) {
if (getBoolArg(platformAndroid)) {
final List<String> devices = await _getDevicesForPlatform('android');
if (devices.isEmpty) {
printError('No Android devices available');
Expand All @@ -99,37 +99,36 @@ class DriveExamplesCommand extends PackageLoopingCommand {
androidDevice = devices.first;
}

String? iosDevice;
if (getBoolArg(kPlatformIos)) {
String? iOSDevice;
if (getBoolArg(platformIOS)) {
final List<String> devices = await _getDevicesForPlatform('ios');
if (devices.isEmpty) {
printError('No iOS devices available');
throw ToolExit(_exitNoAvailableDevice);
}
iosDevice = devices.first;
iOSDevice = devices.first;
}

_targetDeviceFlags = <String, List<String>>{
if (getBoolArg(kPlatformAndroid))
kPlatformAndroid: <String>['-d', androidDevice!],
if (getBoolArg(kPlatformIos)) kPlatformIos: <String>['-d', iosDevice!],
if (getBoolArg(kPlatformLinux)) kPlatformLinux: <String>['-d', 'linux'],
if (getBoolArg(kPlatformMacos)) kPlatformMacos: <String>['-d', 'macos'],
if (getBoolArg(kPlatformWeb))
kPlatformWeb: <String>[
if (getBoolArg(platformAndroid))
platformAndroid: <String>['-d', androidDevice!],
if (getBoolArg(platformIOS)) platformIOS: <String>['-d', iOSDevice!],
if (getBoolArg(platformLinux)) platformLinux: <String>['-d', 'linux'],
if (getBoolArg(platformMacOS)) platformMacOS: <String>['-d', 'macos'],
if (getBoolArg(platformWeb))
platformWeb: <String>[
'-d',
'web-server',
'--web-port=7357',
'--browser-name=chrome',
if (platform.environment.containsKey('CHROME_EXECUTABLE'))
'--chrome-binary=${platform.environment['CHROME_EXECUTABLE']}',
],
if (getBoolArg(kPlatformWindows))
kPlatformWindows: <String>['-d', 'windows'],
if (getBoolArg(platformWindows))
platformWindows: <String>['-d', 'windows'],
// TODO(stuartmorgan): Check these flags once drive supports UWP:
// https://github.com/flutter/flutter/issues/82821
if (getBoolArg(kPlatformWinUwp))
kPlatformWinUwp: <String>['-d', 'winuwp'],
if (getBoolArg(platformWinUwp)) platformWinUwp: <String>['-d', 'winuwp'],
};
}

Expand All @@ -148,9 +147,9 @@ class DriveExamplesCommand extends PackageLoopingCommand {
in _targetDeviceFlags.entries) {
final String platform = entry.key;
String? variant;
if (platform == kPlatformWindows) {
if (platform == platformWindows) {
variant = platformVariantWin32;
} else if (platform == kPlatformWinUwp) {
} else if (platform == platformWinUwp) {
variant = platformVariantWinUwp;
// TODO(stuartmorgan): Remove this once drive supports UWP.
// https://github.com/flutter/flutter/issues/82821
Expand Down
2 changes: 1 addition & 1 deletion script/tool/lib/src/lint_android_command.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ class LintAndroidCommand extends PackageLoopingCommand {

@override
Future<PackageResult> runForPackage(RepositoryPackage package) async {
if (!pluginSupportsPlatform(kPlatformAndroid, package,
if (!pluginSupportsPlatform(platformAndroid, package,
requiredMode: PlatformSupport.inline)) {
return PackageResult.skip(
'Plugin does not have an Android implemenatation.');
Expand Down
Loading