Skip to content

Conversation

@danrubel
Copy link
Contributor

A continuation of #6227 so that flutter doctor can detect IntelliJ product installations and ensure that the needed plugins are installed on Mac.

  • detect IntelliJ on Mac
  • cleanup/refactor Platform.environment['HOME'] accesses to single homeDirPath function

Fixes #5875 and fixes flutter/flutter-intellij#229.
@Hixie

/// Return the absolute path of the user's home directory
String get homeDirPath {
if (_homeDirPath == null) {
_homeDirPath = Platform.environment['HOME'] ?? Platform.environment['USERPROFILE'];
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this should be os specific; in windows we'd use USERPROFILE, and on posix HOME.

Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps use a ternary expression against the os, or add this as a method in the OperatingSystemUtils class?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fair enough. Changed to check Platform.isWindows.


class IntellijValidator extends DoctorValidator {
IntellijValidator(String title, {this.version, this.pluginsPath}) : super(title);
abstract class IntellijValidator extends DoctorValidator {
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: I would capitalize this IntelliJValidator -

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

}
}

class IntellijValidatorOnLinux extends IntellijValidator {
Copy link
Contributor

Choose a reason for hiding this comment

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

perhaps IntelliJLinuxValidator?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

// ignored
}
if (plist != null) {
int index = plist.indexOf('CFBundleShortVersionString');
Copy link
Contributor

Choose a reason for hiding this comment

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

This could move inside the try-catch - you could avoid the plist != null check.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good suggestion. Done.

@devoncarew
Copy link
Contributor

lgtm w/ a few comments -

@danrubel danrubel merged commit e20ee04 into flutter:master Oct 13, 2016
@danrubel danrubel deleted the detect-intellij-on-mac branch October 13, 2016 17:34
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 15, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

flutter doctor should test for IntelliJ not Atom update flutter doctor to show info about installed IDEs

2 participants