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

Show error when Dart SDK is not set correctly (#636) #646

Merged
merged 2 commits into from
Jan 20, 2017
Merged

Conversation

skybrian
Copy link
Contributor

Since the Flutter SDK is derived the from Dart SDK, we actually handle
this by reporting: 'No Flutter SDK configured'. When the user fixes
the Flutter SDK, the Dart SDK will also be fixed.

Fixed combo box to display correctly when no Flutter SDK is configured.
Fire a Flutter SDK changed event when the user changes the Flutter SDK.
Removed dead code.

Not done: when the user changes the Dart SDK, we should fire a Flutter
SDK changed event.

(Sorry about the repost; I accidentally deleted the branch for the last pull request.)

Since the Flutter SDK is derived the from Dart SDK, we actually handle
this by reporting: 'No Flutter SDK configured'. When the user fixes
the Flutter SDK, the Dart SDK will also be fixed.

Fixed combo box to display correctly when no Flutter SDK is configured.
Fire a Flutter SDK changed event when the user changes the Flutter SDK.
Removed dead code.

Not done: when the user changes the Dart SDK, we should fire a Flutter
SDK changed event.
@pq
Copy link
Contributor

pq commented Jan 19, 2017

Cool!

I added a comment to the older PR to the effect that I don't see how we're guarding against the case where a user.

  1. sets the Flutter SDK
  2. updates the Dart SDK (to a different location)

#637 (comment)

Curious what @alexander-doroshko is thinking there.

This is what the old logic to detect mismatched SDKs was intended to catch.

@skybrian
Copy link
Contributor Author

Answered already I think, but I'll answer again here to avoid confusion.

When you set the Dart SDK to a different location, the Flutter SDK is automatically unset (because the Dart SDK changed, and the Flutter SDK is derived from it). However, this wasn't visible to the user due to UI bugs - it would still show the old Flutter SDK even though the setting changed. I've mostly fixed the UI bugs in this patch - in particular see the changes in FlutterSettingsConfigurable.

@pq
Copy link
Contributor

pq commented Jan 20, 2017

Ah! Thanks for the clarification. It was the UI bug that was throwing me off.

Copy link
Contributor

@pq pq left a comment

Choose a reason for hiding this comment

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

LGTM!

*/
private void updateVersionTextIfCurrent(@NotNull FlutterSdk sdk, @NotNull String value) {
FlutterSdk current = FlutterSdk.forPath(getSdkPathText());
if (current == null || !sdk.getHomePath().equals(current.getHomePath())) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe consider Objects.equals(sdk.getHomePath(), current.getHomePath()))? (Gracefully handles nulls -- not that there should be any!)

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.

3 participants