Skip to content
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

Correctly generate Flutter module projects #2504

Merged
merged 8 commits into from Jul 31, 2018
Merged

Correctly generate Flutter module projects #2504

merged 8 commits into from Jul 31, 2018

Conversation

stevemessick
Copy link
Member

@devoncarew @pq

This is still behind a flag, so you won't see modules yet. But this PR includes a complete re-write of project creation, which will affect everyone. We might want to hold this until after M27. Please advise.

Previously, we created a skeletal project and used its window to display flutter create progress in the form of a stdout log. Then, mid-way through project creation we deleted all files and generated all-new files using Flutter. After project creation more-or-less finished we did a few clean-up steps to make the project look right. All that threading and synchrony proved to be too much for module projects, even with more extensive refactoring that what is included here.

With this PR we no longer create a project just to use its logger window. Unfortunately, until we get a new UI implemented, that means there is no feedback during flutter create. That's why I think we might want to label this one WIP and hold it until the new UI is in place (or at least after the next release). The new procedure is to generate the Flutter project files, then open that project in its own window. I think we can add a log view to the final page of the new project wizard for displaying Flutter stdout, but that's going to require more refactoring of base Flutter-plugin code.

The change to AndroidModuleLibraryManager fixes a bug related to Flutter modules. I added a TODO that indicates work is still needed to properly generate Flutter modules that are IntelliJ modules (i.e. contained within an Android app, not our primary use-case).

@stevemessick
Copy link
Member Author

stevemessick commented Jul 27, 2018

The latest commit adds a simple progress indicator around the flutter create process. That may be all that is needed; at least for now it gives as much indication of background processing as we have in many other places. The nice thing is that project creation has less window flashing and seems smoother now.

This means we could, if we are confident that testing is adequate, include this in the next release. Of course, it will need to be reviewed first :)

@devoncarew
Copy link
Member

@pq, I may not have time to review this well - can you take a look?

@pq
Copy link
Contributor

pq commented Jul 31, 2018

Sorry for the slow reply @stevemessick! I keep hoping to have the focus to get enough context back to be really helpful here.

My gut says this feels a bit complex, but then the existing story wasn't really better. (Just the devil I know I guess.) . I think we should just move forward and iterate from there.

That said,

With this PR we no longer create a project just to use its logger window. Unfortunately, until we get a new UI implemented, that means there is no feedback during flutter create. That's why I think we might want to label this one WIP and hold it until the new UI is in place (or at least after the next release).

I agree. Losing that feedback is a step backwards. Have you had any more thoughts in the meantime?

@pq
Copy link
Contributor

pq commented Jul 31, 2018

FWIW: I've been working lately using the EAP. Ping me when this lands and I'll load it and test behavior.

@stevemessick
Copy link
Member Author

Thanks @pq (and @devoncarew). Merging now. I wanted to respond to a couple remarks, too.

I'm pretty sure the old style, using a callback during project creation, was causing intermittent problems due to two factors. Having to reload the project destroyed the original project object, which was still in use in queued-up actions for analysis, highlighting, etc. Errors due to project-already-disposed are the main symptom of that.

Related to that, the multiple execution queues were also causing problems due to their unpredictability. We had stuff executing in main thread, EDT, background sync, and background async. I lost track of what was happening when. And I strongly suspect timing issues could affect scheduling; if true, that would explain some hard-to-interpret anomalies, such as a Project view that did not show Dart files or possibly the symptom in #2471.

Concerning UI feedback, I think the indeterminate progress indicator I added does a decent job. I did want to talk with you about doing a fairly large refactoring to allow the console log view to be included as a parameter in the flutter create job but I'm not even convinced it is needed.

We can still decide whether this is part of M27 or not. It would need to be cherry-picked to include it. I think we should at least test a build that includes it.

@stevemessick stevemessick merged commit 7d3f4ca into master Jul 31, 2018
@pq
Copy link
Contributor

pq commented Jul 31, 2018

Thanks for all the context. Sounds great!

@stevemessick stevemessick deleted the add2app branch August 14, 2018 20:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants