[flutter_tools] Ensure flutter daemon clients can detect preview device #140112
Conversation
github-actions
bot
added
the
tool
christopherfujino
force-pushed
the
upload-preview-artifact
branch
2 times, most recently
from
6ca846c to
1dd5162
Compare
christopherfujino
changed the title
packages/flutter_tools/test/commands.shard/hermetic/daemon_test.dart
Outdated
Show resolved
Hide resolved
christopherfujino
force-pushed
the
upload-preview-artifact
branch
from
d6ec9fe to
7171e31
Compare
|
@christopherfujino I'm still a little confused about what the But in this code we're adding If the intention is to use However, I'm not sure whether using "windows" is the best option if we want to be able to conditionally show this per-project. We can't change which devices are available per-project, only filter by device-type. So the Preview device will always be sent to VS Code (when enabled) regardless of project capabilities, so if we want to be able to control its visibility (eg. to show it only when a project doesn't have plugins), I think it would need its own platform type. For example maybe the preview device should have a platformType of |
christopherfujino
force-pushed
the
upload-preview-artifact
branch
from
7171e31 to
224e6a6
Compare
| @@ -416,36 +416,129 @@ class DaemonDomain extends Domain { | |||
| /// is correct. | |||
| Future<Map<String, Object>> getSupportedPlatforms(Map<String, Object?> args) async { | |||
| final String? projectRoot = _getStringArg(args, 'projectRoot', required: true); | |||
| final List<String> result = <String>[]; | |||
| final List<String> platforms = <String>[]; | |||
There was a problem hiding this comment.
nit: should we rename some of these variables (that aren't part of the API) platformType and/or add a comment to make it a bit clearer that almost all code here is platform types and not platforms?
There was a problem hiding this comment.
I will change this variable name, but I would add that--in my mind--the situation is far more confusing than just that we use the wrong terminology here.
I don't think the tool has a standardized concept of what exactly a target platform is. It is probably roughly something like operating system + device architecture, but it's complicated by for example the web (js vs WASM), the tester device, the preview device, and custom devices. In particular, there is a lot of complexity in the build_info.dart library: https://github.com/flutter/flutter/blob/master/packages/flutter_tools/lib/src/build_info.dart#L509
Similarly, even though we have an enum named PlatformType, I wouldn't call that a standardized concept in the tool.
| } | ||
| if (!supportedPlatforms.contains(SupportedPlatform.linux)) { | ||
| isSupported = false; | ||
| reasons.add('the current Flutter project does not have a Linux platform directory'); |
There was a problem hiding this comment.
Would something like "The Linux platform is not currently enabled for this project" read better? I'm thinking of when it's visible in the UI:
DEVICES
-------------------------------------------------------------------------- Available Devices
Chrome
Edge
-------------------------------------------------------------------------- Unavailable Devices
Enable Linux
The Linux platform is not currently enabled for this project
Enable macOS
The macOS platform is not enabled globally
Part of #130277