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

fix feature break from October VSCode insider #87

Merged
merged 4 commits into from Oct 16, 2017
Merged

Conversation

testforstephen
Copy link
Contributor

Signed-off-by: Jinbo Wang jinbwan@microsoft.com

#82

Signed-off-by: Jinbo Wang <jinbwan@microsoft.com>
import TelemetryReporter from "vscode-extension-telemetry";
import * as commands from "./commands";

export class JavaConfigurationProvider implements vscode.DebugConfigurationProvider {
Copy link
Contributor

Choose a reason for hiding this comment

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

JavaDebugConfigurationProvider

}

// Try to add all missing attributes to the debug configuration being launched.
public resolveDebugConfiguration(folder: vscode.WorkspaceFolder | undefined, config: vscode.DebugConfiguration, token?: vscode.CancellationToken):
Copy link
Contributor

Choose a reason for hiding this comment

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

This method is not marked async yet call async meethods

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is VSCode API built-in methods. No need to change it to async. because the result vscode.ProviderResult<vscode.DebugConfiguration> could be T or Thenable<T>.
Besides, changing the method declaration to async will ask us to modify the return value to Promise<T>, too.

Copy link
Contributor

Choose a reason for hiding this comment

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

That's the point. If you mark as async, then the caller will know how to deal with it. If you don't provide the same behavior as method signature, the behavior will be undefined in some cases.

// Try to add all missing attributes to the debug configuration being launched.
public resolveDebugConfiguration(folder: vscode.WorkspaceFolder | undefined, config: vscode.DebugConfiguration, token?: vscode.CancellationToken):
vscode.ProviderResult<vscode.DebugConfiguration> {
return this.guessAndValidateDebugConfiguration(folder, config);
Copy link
Contributor

@yaohaizh yaohaizh Oct 11, 2017

Choose a reason for hiding this comment

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

Normally in this sitation, we will call method heuristic....

@testforstephen
Copy link
Contributor Author

VSCode will drop these debug config items in the following October milestone:
microsoft/vscode#33791
microsoft/vscode#33794
microsoft/vscode#33795

config.classPaths = await resolveClasspath(config.mainClass, config.projectName);
}
if (!config.classPaths || !Array.isArray(config.classPaths) || !config.classPaths.length) {
vscode.window.showErrorMessage("Cannot resolve the classpaths automatically, please specify the value in the launch.json.");
Copy link
Contributor

Choose a reason for hiding this comment

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

log error?

Signed-off-by: Jinbo Wang <jinbwan@microsoft.com>
…ow exception for Object

Signed-off-by: Jinbo Wang <jinbwan@microsoft.com>
Signed-off-by: Jinbo Wang <jinbwan@microsoft.com>
@microsoft microsoft deleted a comment from msftclas Oct 15, 2017
@testforstephen testforstephen merged commit e08d017 into master Oct 16, 2017
@testforstephen testforstephen deleted the jinbo_oct branch October 16, 2017 03:55
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.

None yet

5 participants