-
Notifications
You must be signed in to change notification settings - Fork 330
more work to support running Flutter web apps #3389
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
Conversation
@Nullable FlutterDevice device = DeviceService.getInstance(project).getSelectedDevice(); | ||
|
||
boolean isFlutterWeb = false; | ||
final String filePath = ((SdkRunConfig)runConfig).getFields().getFilePath(); |
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.
Do you need to normalize here to ensure windows is supported, or has that already happened in getFilePath?
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.
This just restructures the code on the left to remove a level of nesting - it's fundamentally the same as the previous code.
Some updates:
With the latest updates, the inspector now works. |
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.
2 nits, but LGTM
catch (UnsupportedEncodingException ignored) { | ||
} | ||
|
||
BrowserLauncher.getInstance().browse( |
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.
Do you want browse()
to be called even if there is a caught exception in the try block above? Reading the code here makes it look like browse()
here is supposed to be inside the try-block.
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.
good point; this shouldn't happen - the UTF-8 encoding should be available - but I'll move the browser call up to within the try block in any case
} | ||
catch (Exception e) { | ||
// Nothing to do if the service can't be acquired. | ||
} |
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.
e -> ignored for IntelliJ inspections.
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.
done
This reverts commit ff4e5ff.
* clean up to our flutter web support * remove printlns * fix inspector npes * review comments
Various work in support of #3293:
"Build: Running build completed, took 191ms"
) was erroneously hyperlinked to in the run consoledaemon.log
event, to better support showing output from running flutter web appsFlutterCommand.FLUTTER_WEB_RUN
commandThings that are not yet supported:
Both of the above issues will need to be resolved for flutter web support (not necessarily in this PR).