Unverified Commit e5414695 authored by Ian Hickson's avatar Ian Hickson Committed by GitHub

Change --disable-dds to --no-dds to avoid double negatives (#80900)

Also, refactor internal code to do the same.

See https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo#avoid-double-negatives-in-apis
parent 7ec7d4da
...@@ -430,7 +430,7 @@ known, it can be explicitly provided to attach via the command-line, e.g. ...@@ -430,7 +430,7 @@ known, it can be explicitly provided to attach via the command-line, e.g.
final List<FlutterDevice> flutterDevices = <FlutterDevice>[flutterDevice]; final List<FlutterDevice> flutterDevices = <FlutterDevice>[flutterDevice];
final DebuggingOptions debuggingOptions = DebuggingOptions.enabled( final DebuggingOptions debuggingOptions = DebuggingOptions.enabled(
buildInfo, buildInfo,
disableDds: boolArg('disable-dds'), enableDds: enableDds,
devToolsServerAddress: devToolsServerAddress, devToolsServerAddress: devToolsServerAddress,
); );
......
...@@ -48,7 +48,7 @@ class DevicesCommand extends FlutterCommand { ...@@ -48,7 +48,7 @@ class DevicesCommand extends FlutterCommand {
@override @override
Future<void> validateCommand() { Future<void> validateCommand() {
if (argResults['timeout'] != null) { if (argResults['timeout'] != null) {
globals.printError('"--timeout" argument is deprecated, use "--${FlutterOptions.kDeviceTimeout}" instead'); globals.printError('${globals.logger.terminal.warningMark} The "--timeout" argument is deprecated; use "--${FlutterOptions.kDeviceTimeout}" instead.');
} }
return super.validateCommand(); return super.validateCommand();
} }
......
...@@ -191,7 +191,7 @@ abstract class RunCommandBase extends FlutterCommand with DeviceBasedDevelopment ...@@ -191,7 +191,7 @@ abstract class RunCommandBase extends FlutterCommand with DeviceBasedDevelopment
buildInfo, buildInfo,
startPaused: boolArg('start-paused'), startPaused: boolArg('start-paused'),
disableServiceAuthCodes: boolArg('disable-service-auth-codes'), disableServiceAuthCodes: boolArg('disable-service-auth-codes'),
disableDds: boolArg('disable-dds'), enableDds: enableDds,
dartEntrypointArgs: stringsArg('dart-entrypoint-args'), dartEntrypointArgs: stringsArg('dart-entrypoint-args'),
dartFlags: stringArg('dart-flags') ?? '', dartFlags: stringArg('dart-flags') ?? '',
useTestFonts: argParser.options.containsKey('use-test-fonts') && boolArg('use-test-fonts'), useTestFonts: argParser.options.containsKey('use-test-fonts') && boolArg('use-test-fonts'),
......
...@@ -392,7 +392,7 @@ class TestCommand extends FlutterCommand with DeviceBasedDevelopmentArtifacts { ...@@ -392,7 +392,7 @@ class TestCommand extends FlutterCommand with DeviceBasedDevelopmentArtifacts {
disableServiceAuthCodes: boolArg('disable-service-auth-codes'), disableServiceAuthCodes: boolArg('disable-service-auth-codes'),
// On iOS >=14, keeping this enabled will leave a prompt on the screen. // On iOS >=14, keeping this enabled will leave a prompt on the screen.
disablePortPublication: true, disablePortPublication: true,
disableDds: disableDds, enableDds: enableDds,
nullAssertions: boolArg(FlutterOptions.kNullAssertions), nullAssertions: boolArg(FlutterOptions.kNullAssertions),
); );
......
...@@ -717,7 +717,7 @@ class DebuggingOptions { ...@@ -717,7 +717,7 @@ class DebuggingOptions {
this.buildInfo, { this.buildInfo, {
this.startPaused = false, this.startPaused = false,
this.disableServiceAuthCodes = false, this.disableServiceAuthCodes = false,
this.disableDds = false, this.enableDds = true,
this.dartEntrypointArgs = const <String>[], this.dartEntrypointArgs = const <String>[],
this.dartFlags = '', this.dartFlags = '',
this.enableSoftwareRendering = false, this.enableSoftwareRendering = false,
...@@ -768,7 +768,7 @@ class DebuggingOptions { ...@@ -768,7 +768,7 @@ class DebuggingOptions {
startPaused = false, startPaused = false,
dartFlags = '', dartFlags = '',
disableServiceAuthCodes = false, disableServiceAuthCodes = false,
disableDds = false, enableDds = true,
enableSoftwareRendering = false, enableSoftwareRendering = false,
skiaDeterministicRendering = false, skiaDeterministicRendering = false,
traceSkia = false, traceSkia = false,
...@@ -795,7 +795,7 @@ class DebuggingOptions { ...@@ -795,7 +795,7 @@ class DebuggingOptions {
final String dartFlags; final String dartFlags;
final List<String> dartEntrypointArgs; final List<String> dartEntrypointArgs;
final bool disableServiceAuthCodes; final bool disableServiceAuthCodes;
final bool disableDds; final bool enableDds;
final bool enableSoftwareRendering; final bool enableSoftwareRendering;
final bool skiaDeterministicRendering; final bool skiaDeterministicRendering;
final bool traceSkia; final bool traceSkia;
......
...@@ -270,7 +270,7 @@ class ResidentWebRunner extends ResidentRunner { ...@@ -270,7 +270,7 @@ class ResidentWebRunner extends ResidentRunner {
useSseForInjectedClient: debuggingOptions.webUseSseForInjectedClient, useSseForInjectedClient: debuggingOptions.webUseSseForInjectedClient,
buildInfo: debuggingOptions.buildInfo, buildInfo: debuggingOptions.buildInfo,
enableDwds: _enableDwds, enableDwds: _enableDwds,
enableDds: !debuggingOptions.disableDds, enableDds: debuggingOptions.enableDds,
entrypoint: _fileSystem.file(target).uri, entrypoint: _fileSystem.file(target).uri,
expressionCompiler: expressionCompiler, expressionCompiler: expressionCompiler,
chromiumLauncher: _chromiumLauncher, chromiumLauncher: _chromiumLauncher,
......
...@@ -231,7 +231,7 @@ class FlutterDevice { ...@@ -231,7 +231,7 @@ class FlutterDevice {
int hostVmServicePort, int hostVmServicePort,
int ddsPort, int ddsPort,
bool disableServiceAuthCodes = false, bool disableServiceAuthCodes = false,
bool disableDds = false, bool enableDds = true,
@required bool allowExistingDdsInstance, @required bool allowExistingDdsInstance,
bool ipv6 = false, bool ipv6 = false,
}) { }) {
...@@ -245,7 +245,7 @@ class FlutterDevice { ...@@ -245,7 +245,7 @@ class FlutterDevice {
isWaitingForVm = true; isWaitingForVm = true;
bool existingDds = false; bool existingDds = false;
FlutterVmService service; FlutterVmService service;
if (!disableDds) { if (enableDds) {
void handleError(Exception e, StackTrace st) { void handleError(Exception e, StackTrace st) {
globals.printTrace('Fail to connect to service protocol: $observatoryUri: $e'); globals.printTrace('Fail to connect to service protocol: $observatoryUri: $e');
if (!completer.isCompleted) { if (!completer.isCompleted) {
...@@ -300,7 +300,7 @@ class FlutterDevice { ...@@ -300,7 +300,7 @@ class FlutterDevice {
service = await Future.any<dynamic>( service = await Future.any<dynamic>(
<Future<dynamic>>[ <Future<dynamic>>[
connectToVmService( connectToVmService(
disableDds ? observatoryUri : device.dds.uri, enableDds ? device.dds.uri : observatoryUri,
reloadSources: reloadSources, reloadSources: reloadSources,
restart: restart, restart: restart,
compileExpression: compileExpression, compileExpression: compileExpression,
...@@ -1338,7 +1338,7 @@ abstract class ResidentRunner extends ResidentHandlers { ...@@ -1338,7 +1338,7 @@ abstract class ResidentRunner extends ResidentHandlers {
reloadSources: reloadSources, reloadSources: reloadSources,
restart: restart, restart: restart,
compileExpression: compileExpression, compileExpression: compileExpression,
disableDds: debuggingOptions.disableDds, enableDds: debuggingOptions.enableDds,
ddsPort: debuggingOptions.ddsPort, ddsPort: debuggingOptions.ddsPort,
allowExistingDdsInstance: allowExistingDdsInstance, allowExistingDdsInstance: allowExistingDdsInstance,
hostVmServicePort: debuggingOptions.hostVmServicePort, hostVmServicePort: debuggingOptions.hostVmServicePort,
......
...@@ -370,18 +370,46 @@ abstract class FlutterCommand extends Command<void> { ...@@ -370,18 +370,46 @@ abstract class FlutterCommand extends Command<void> {
'Specifying port 0 (the default) will find a random free port.' 'Specifying port 0 (the default) will find a random free port.'
); );
argParser.addFlag( argParser.addFlag(
'disable-dds', // TODO(ianh): this should be called `dds` and default to true (see style guide about double negatives) 'dds',
hide: !verboseHelp, hide: !verboseHelp,
help: 'Disable the Dart Developer Service (DDS). This flag should only be provided ' defaultsTo: true,
'when attaching to an application with an existing DDS instance (e.g., ' help: 'Enable the Dart Developer Service (DDS).\n'
'attaching to an application currently connected to by "flutter run") or ' 'It may be necessary to disable this when attaching to an application with '
'when running certain tests.\n' 'an existing DDS instance (e.g., attaching to an application currently '
'Passing this flag may degrade IDE functionality if a DDS instance is not ' 'connected to by "flutter run"), or when running certain tests.\n'
'already connected to the target application.' 'Disabling this feature may degrade IDE functionality if a DDS instance is '
'not already connected to the target application.'
);
argParser.addFlag(
'disable-dds',
hide: !verboseHelp,
help: '(deprecated; use "--no-dds" instead) '
'Disable the Dart Developer Service (DDS).'
); );
} }
bool get disableDds => boolArg('disable-dds'); bool _ddsEnabled;
bool get enableDds {
if (_ddsEnabled == null) {
if (argResults.wasParsed('disable-dds')) {
if (argResults.wasParsed('dds')) {
throwToolExit('The "--[no-]dds" and "--[no-]disable-dds" arguments are mutually exclusive. Only specify "--[no-]dds".');
}
_ddsEnabled = !boolArg('disable-dds');
// TODO(ianh): enable the following code once google3 is migrated away from --disable-dds (and add test to flutter_command_test.dart)
if (false) { // ignore: dead_code
if (_ddsEnabled) {
globals.printError('${globals.logger.terminal.warningMark} The "--no-disable-dds" argument is deprecated and redundant, and should be omitted.');
} else {
globals.printError('${globals.logger.terminal.warningMark} The "--disable-dds" argument is deprecated. Use "--no-dds" instead.');
}
}
} else {
_ddsEnabled = boolArg('dds');
}
}
return _ddsEnabled;
}
bool get _hostVmServicePortProvided => argResults.wasParsed('observatory-port') || bool get _hostVmServicePortProvided => argResults.wasParsed('observatory-port') ||
argResults.wasParsed('host-vmservice-port'); argResults.wasParsed('host-vmservice-port');
...@@ -435,7 +463,7 @@ abstract class FlutterCommand extends Command<void> { ...@@ -435,7 +463,7 @@ abstract class FlutterCommand extends Command<void> {
// If DDS is enabled and no explicit DDS port is provided, use the // If DDS is enabled and no explicit DDS port is provided, use the
// host-vmservice-port for DDS instead and bind the VM service to a random // host-vmservice-port for DDS instead and bind the VM service to a random
// port. // port.
if (!disableDds && !argResults.wasParsed('dds-port')) { if (enableDds && !argResults.wasParsed('dds-port')) {
return null; return null;
} }
return _tryParseHostVmservicePort(); return _tryParseHostVmservicePort();
...@@ -460,8 +488,9 @@ abstract class FlutterCommand extends Command<void> { ...@@ -460,8 +488,9 @@ abstract class FlutterCommand extends Command<void> {
negatable: true, negatable: true,
hide: !verboseHelp, hide: !verboseHelp,
help: 'Publish the VM service port over mDNS. Disable to prevent the ' help: 'Publish the VM service port over mDNS. Disable to prevent the '
'local network permission app dialog in debug and profile build modes (iOS devices only.)', 'local network permission app dialog in debug and profile build modes (iOS devices only).',
defaultsTo: enabledByDefault); defaultsTo: enabledByDefault,
);
} }
bool get disablePortPublication => !boolArg('publish-port'); bool get disablePortPublication => !boolArg('publish-port');
...@@ -1068,11 +1097,13 @@ abstract class FlutterCommand extends Command<void> { ...@@ -1068,11 +1097,13 @@ abstract class FlutterCommand extends Command<void> {
void _printDeprecationWarning() { void _printDeprecationWarning() {
if (deprecated) { if (deprecated) {
globals.printStatus('${globals.logger.terminal.warningMark} The "$name" command is deprecated and ' globals.printError(
'${globals.logger.terminal.warningMark} The "$name" command is deprecated and '
'will be removed in a future version of Flutter. ' 'will be removed in a future version of Flutter. '
'See https://flutter.dev/docs/development/tools/sdk/releases ' 'See https://flutter.dev/docs/development/tools/sdk/releases '
'for previous releases of Flutter.'); 'for previous releases of Flutter.',
globals.printStatus(''); );
globals.printError('');
} }
} }
......
...@@ -98,7 +98,7 @@ class FlutterTesterTestDevice extends TestDevice { ...@@ -98,7 +98,7 @@ class FlutterTesterTestDevice extends TestDevice {
// //
// I mention this only so that you won't be tempted, as I was, to apply // I mention this only so that you won't be tempted, as I was, to apply
// the obvious simplification to this code and remove this entire feature. // the obvious simplification to this code and remove this entire feature.
'--observatory-port=${debuggingOptions.disableDds ? debuggingOptions.hostVmServicePort: 0}', '--observatory-port=${debuggingOptions.enableDds ? 0 : debuggingOptions.hostVmServicePort }',
if (debuggingOptions.startPaused) '--start-paused', if (debuggingOptions.startPaused) '--start-paused',
if (debuggingOptions.disableServiceAuthCodes) '--disable-service-auth-codes', if (debuggingOptions.disableServiceAuthCodes) '--disable-service-auth-codes',
] ]
...@@ -158,7 +158,7 @@ class FlutterTesterTestDevice extends TestDevice { ...@@ -158,7 +158,7 @@ class FlutterTesterTestDevice extends TestDevice {
debuggingOptions.hostVmServicePort == detectedUri.port); debuggingOptions.hostVmServicePort == detectedUri.port);
Uri forwardingUri; Uri forwardingUri;
if (!debuggingOptions.disableDds) { if (debuggingOptions.enableDds) {
logger.printTrace('test $id: Starting Dart Development Service'); logger.printTrace('test $id: Starting Dart Development Service');
final DartDevelopmentService dds = await startDds(detectedUri); final DartDevelopmentService dds = await startDds(detectedUri);
forwardingUri = dds.uri; forwardingUri = dds.uri;
......
...@@ -206,7 +206,7 @@ class TestFlutterDevice extends FlutterDevice { ...@@ -206,7 +206,7 @@ class TestFlutterDevice extends FlutterDevice {
CompileExpression compileExpression, CompileExpression compileExpression,
GetSkSLMethod getSkSLMethod, GetSkSLMethod getSkSLMethod,
PrintStructuredErrorLogMethod printStructuredErrorLogMethod, PrintStructuredErrorLogMethod printStructuredErrorLogMethod,
bool disableDds = false, bool enableDds = true,
bool disableServiceAuthCodes = false, bool disableServiceAuthCodes = false,
int hostVmServicePort, int hostVmServicePort,
int ddsPort, int ddsPort,
......
...@@ -231,7 +231,7 @@ void main() { ...@@ -231,7 +231,7 @@ void main() {
observatoryUri: Uri.parse('http://127.0.0.1:63426/1UasC_ihpXY=/'), observatoryUri: Uri.parse('http://127.0.0.1:63426/1UasC_ihpXY=/'),
)); ));
await driverService.start(BuildInfo.profile, device, DebuggingOptions.enabled(BuildInfo.profile, disableDds: true), true); await driverService.start(BuildInfo.profile, device, DebuggingOptions.enabled(BuildInfo.profile, enableDds: false), true);
final int testResult = await driverService.startTest( final int testResult = await driverService.startTest(
'foo.test', 'foo.test',
<String>[], <String>[],
......
...@@ -221,7 +221,7 @@ class TestFlutterTesterDevice extends FlutterTesterTestDevice { ...@@ -221,7 +221,7 @@ class TestFlutterTesterDevice extends FlutterTesterTestDevice {
packagesPath: '.dart_tool/package_config.json', packagesPath: '.dart_tool/package_config.json',
), ),
startPaused: false, startPaused: false,
disableDds: false, enableDds: true,
disableServiceAuthCodes: false, disableServiceAuthCodes: false,
hostVmServicePort: 1234, hostVmServicePort: 1234,
nullAssertions: false, nullAssertions: false,
......
...@@ -366,7 +366,7 @@ class TestFlutterDevice extends FlutterDevice { ...@@ -366,7 +366,7 @@ class TestFlutterDevice extends FlutterDevice {
GetSkSLMethod getSkSLMethod, GetSkSLMethod getSkSLMethod,
PrintStructuredErrorLogMethod printStructuredErrorLogMethod, PrintStructuredErrorLogMethod printStructuredErrorLogMethod,
bool disableServiceAuthCodes = false, bool disableServiceAuthCodes = false,
bool disableDds = false, bool enableDds = true,
bool ipv6 = false, bool ipv6 = false,
int hostVmServicePort, int hostVmServicePort,
int ddsPort, int ddsPort,
......
...@@ -2517,7 +2517,7 @@ class FakeFlutterDevice extends FlutterDevice { ...@@ -2517,7 +2517,7 @@ class FakeFlutterDevice extends FlutterDevice {
Future<void> connect({ Future<void> connect({
ReloadSources reloadSources, ReloadSources reloadSources,
Restart restart, Restart restart,
bool disableDds = false, bool enableDds = true,
bool disableServiceAuthCodes = false, bool disableServiceAuthCodes = false,
bool ipv6 = false, bool ipv6 = false,
CompileExpression compileExpression, CompileExpression compileExpression,
......
...@@ -87,7 +87,7 @@ void main() { ...@@ -87,7 +87,7 @@ void main() {
final CommandRunner<void> runner = createTestCommandRunner(flutterCommand); final CommandRunner<void> runner = createTestCommandRunner(flutterCommand);
await runner.run(<String>['deprecated']); await runner.run(<String>['deprecated']);
expect(testLogger.statusText, expect(testLogger.errorText,
contains('The "deprecated" command is deprecated and will be removed in ' contains('The "deprecated" command is deprecated and will be removed in '
'a future version of Flutter.')); 'a future version of Flutter.'));
expect(flutterCommand.usage, expect(flutterCommand.usage,
...@@ -546,6 +546,49 @@ void main() { ...@@ -546,6 +546,49 @@ void main() {
final BuildInfo buildInfo = await flutterCommand.getBuildInfo(forcedBuildMode: BuildMode.debug); final BuildInfo buildInfo = await flutterCommand.getBuildInfo(forcedBuildMode: BuildMode.debug);
expect(buildInfo.packagesPath, 'foo'); expect(buildInfo.packagesPath, 'foo');
}); });
testUsingContext('dds options', () async {
final FakeDdsCommand ddsCommand = FakeDdsCommand();
final CommandRunner<void> runner = createTestCommandRunner(ddsCommand);
await runner.run(<String>['test', '--dds-port=1']);
expect(ddsCommand.enableDds, isTrue);
expect(ddsCommand.ddsPort, 1);
});
testUsingContext('dds options --dds', () async {
final FakeDdsCommand ddsCommand = FakeDdsCommand();
final CommandRunner<void> runner = createTestCommandRunner(ddsCommand);
await runner.run(<String>['test', '--dds']);
expect(ddsCommand.enableDds, isTrue);
});
testUsingContext('dds options --no-dds', () async {
final FakeDdsCommand ddsCommand = FakeDdsCommand();
final CommandRunner<void> runner = createTestCommandRunner(ddsCommand);
await runner.run(<String>['test', '--no-dds']);
expect(ddsCommand.enableDds, isFalse);
});
testUsingContext('dds options --disable-dds', () async {
final FakeDdsCommand ddsCommand = FakeDdsCommand();
final CommandRunner<void> runner = createTestCommandRunner(ddsCommand);
await runner.run(<String>['test', '--disable-dds']);
expect(ddsCommand.enableDds, isFalse);
});
testUsingContext('dds options --no-disable-dds', () async {
final FakeDdsCommand ddsCommand = FakeDdsCommand();
final CommandRunner<void> runner = createTestCommandRunner(ddsCommand);
await runner.run(<String>['test', '--no-disable-dds']);
expect(ddsCommand.enableDds, isTrue);
});
testUsingContext('dds options --dds --disable-dds', () async {
final FakeDdsCommand ddsCommand = FakeDdsCommand();
final CommandRunner<void> runner = createTestCommandRunner(ddsCommand);
await runner.run(<String>['test', '--dds', '--disable-dds']);
expect(() => ddsCommand.enableDds, throwsToolExit());
});
}); });
} }
...@@ -611,6 +654,23 @@ class FakeReportingNullSafetyCommand extends FlutterCommand { ...@@ -611,6 +654,23 @@ class FakeReportingNullSafetyCommand extends FlutterCommand {
} }
} }
class FakeDdsCommand extends FlutterCommand {
FakeDdsCommand() {
addDdsOptions(verboseHelp: false);
}
@override
String get description => 'test';
@override
String get name => 'test';
@override
Future<FlutterCommandResult> runCommand() async {
return FlutterCommandResult.success();
}
}
class MockProcessInfo extends Mock implements ProcessInfo {} class MockProcessInfo extends Mock implements ProcessInfo {}
class MockIoProcessSignal extends Mock implements io.ProcessSignal {} class MockIoProcessSignal extends Mock implements io.ProcessSignal {}
......
...@@ -474,7 +474,7 @@ class FlutterRunTestDriver extends FlutterTestDriver { ...@@ -474,7 +474,7 @@ class FlutterRunTestDriver extends FlutterTestDriver {
if (!chrome) if (!chrome)
'--disable-service-auth-codes', '--disable-service-auth-codes',
'--machine', '--machine',
if (!spawnDdsInstance) '--disable-dds', if (!spawnDdsInstance) '--no-dds',
...getLocalEngineArguments(), ...getLocalEngineArguments(),
'-d', '-d',
if (chrome) if (chrome)
...@@ -512,7 +512,7 @@ class FlutterRunTestDriver extends FlutterTestDriver { ...@@ -512,7 +512,7 @@ class FlutterRunTestDriver extends FlutterTestDriver {
...getLocalEngineArguments(), ...getLocalEngineArguments(),
'--machine', '--machine',
if (!spawnDdsInstance) if (!spawnDdsInstance)
'--disable-dds', '--no-dds',
'-d', '-d',
'flutter-tester', 'flutter-tester',
'--debug-port', '--debug-port',
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment