Skip to content

add the ability to install and open DevTools #3207

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Feb 19, 2019
Merged

add the ability to install and open DevTools #3207

merged 2 commits into from
Feb 19, 2019

Conversation

devoncarew
Copy link
Member

Add the ability to install and open DevTools:

  • add a toolbar action to open devtools from the run and debug consoles
  • add a toolbar action to open devtools from the flutter inspector view
  • the first time in a session the action is run, pub global activate the devtools packags
  • run one devtools server instance to serve the web app; if it's not running when the 'open devtools' action is invoked, start it; that server instance will run for the life of the IntelliJ session
  • fix have a way to launch DevTools from IntelliJ #3151

screen shot 2019-02-18 at 4 37 16 pm

screen shot 2019-02-12 at 1 11 13 pm

screen shot 2019-02-12 at 1 11 07 pm

stevemessick
stevemessick approved these changes Feb 19, 2019
View reviewed changes
src/io/flutter/devtools/DevToolsManager.java
public void onCancel() {
if (processHandler != null && !processHandler.isProcessTerminated()) {
processHandler.destroyProcess();
}
Copy link
Member

Choose a reason for hiding this comment

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

Add "result.complete(false);" ? Can't remember if "processHandler.waitFor()" will return false if canceled.

Copy link
Member Author

Choose a reason for hiding this comment

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

I added this (and guarded against completing something that's already completed).

src/io/flutter/devtools/DevToolsManager.java Outdated
public CompletableFuture<Boolean> installDevTools() {
final FlutterSdk sdk = FlutterSdk.getFlutterSdk(project);
if (sdk == null) {
final CompletableFuture<Boolean> result = new CompletableFuture<>();
Copy link
Member

Choose a reason for hiding this comment

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

Lines 58-60 are frequently repeated. Could do a helper like "return falseLater();"

Copy link
Member Author

Choose a reason for hiding this comment

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

Good point; done (created createCompletedFuture(boolean value)).

src/io/flutter/devtools/DevToolsManager.java Outdated

private boolean installedDevTools = false;

DevToolsInstance devToolsInstance;
Copy link
Member

Choose a reason for hiding this comment

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

Make this private.

Copy link
Member Author

Choose a reason for hiding this comment

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

done

src/io/flutter/view/FlutterView.java Outdated
}
}

class OpenDevToolsAction extends FlutterViewAction {
Copy link
Member

Choose a reason for hiding this comment

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

Can this be unified with io.flutter.run.OpenDevToolsAction? If not, I'd prefer a different name for one of these action classes. E.g. OpenDevToolsMenuAction here or OpenDevToolsToolbarAction there.

Copy link
Member Author

Choose a reason for hiding this comment

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

There is a bunch of overlap. It's slightly easier to have code dupe for these two classes, rather than trying to unify them; I renamed the one specific to the Flutter view to FlutterViewDevToolsAction.

devoncarew
devoncarew commented Feb 19, 2019
View reviewed changes
Copy link
Member Author

@devoncarew devoncarew left a comment

Choose a reason for hiding this comment

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

Thanks for the review! Updated.

src/io/flutter/devtools/DevToolsManager.java Outdated

private boolean installedDevTools = false;

DevToolsInstance devToolsInstance;
Copy link
Member Author

Choose a reason for hiding this comment

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

done

src/io/flutter/devtools/DevToolsManager.java Outdated
public CompletableFuture<Boolean> installDevTools() {
final FlutterSdk sdk = FlutterSdk.getFlutterSdk(project);
if (sdk == null) {
final CompletableFuture<Boolean> result = new CompletableFuture<>();
Copy link
Member Author

Choose a reason for hiding this comment

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

Good point; done (created createCompletedFuture(boolean value)).

src/io/flutter/devtools/DevToolsManager.java
public void onCancel() {
if (processHandler != null && !processHandler.isProcessTerminated()) {
processHandler.destroyProcess();
}
Copy link
Member Author

Choose a reason for hiding this comment

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

I added this (and guarded against completing something that's already completed).

src/io/flutter/view/FlutterView.java Outdated
}
}

class OpenDevToolsAction extends FlutterViewAction {
Copy link
Member Author

Choose a reason for hiding this comment

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

There is a bunch of overlap. It's slightly easier to have code dupe for these two classes, rather than trying to unify them; I renamed the one specific to the Flutter view to FlutterViewDevToolsAction.

@stevemessick
Copy link
Member

LGTM :)

@devoncarew devoncarew merged commit 1856955 into flutter:master Feb 19, 2019
alexander-doroshko pushed a commit to alexander-doroshko/flutter-intellij that referenced this pull request Jan 24, 2020
@devoncarew @alexander-doroshko
add the ability to install and open DevTools (flutter#3207)
09ff603 
* add the ability to install and open DevTools

* review comments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@stevemessick stevemessick stevemessick approved these changes

Assignees

No one assigned

Labels

cla: yes

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

have a way to launch DevTools from IntelliJ

3 participants

@devoncarew @stevemessick @googlebot