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

Updates to version and path parsing to support windows #4

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

JamesCaska
Copy link

Tweaks I found I needed to make it work with windows.

@@ -311,6 +308,15 @@ public void readSettings(WizardDescriptor settings) {
view.platformLocationField.setText( currentPlatform.getRootPath().toString() );
}

if( currentPlatform==null){
resolvePlatformFromPath();
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not sure whether I understand the reason for this change. If the "currentPlatform" isn't non-null at this point, we are risking a NPE a few lines above (308). We could however move this logic to the line right after line 306 where we set the text for view.platformLocationField.

@JamesCaska
Copy link
Author

Hi, thanks for looking into this.

I also don't fully understand the resolving logic. I suppose both currentPlatform was not being set and was being used later in the dropdown dialog without a null check so I added a last chance check.
Your right about the NPE exception risk and that existed before the change. Maybe the resolvePlatformFromPath should move up before if (platformCoreDir != null) { and also add a null check on view.platformLocationField.setText( currentPlatform.getRootPath().toString() );

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants