Skip to content

/ java-debug

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

Jump to bottom

1. add user error in DebugException #208

Merged
merged 14 commits into from Aug 22, 2018
Merged

1. add user error in DebugException #208

merged 14 commits into from Aug 22, 2018

Conversation

@andxu
Copy link
Collaborator

@andxu andxu commented Aug 20, 2018

  1. add user error logic in evaluation since a lot of them are reported in evaluation which is caused by known reasons(eg: compilation error).
  2. enable evaluation on hover
@andxu
1. add user error in DebugException
Loading status checks…
956ad3e 
2. add user error logic in evaluation since a lot of them are reported in evaluation which is caused by known reasons(eg: compilation error).
3. enable evaluation on hover
@testforstephen
testforstephen requested changes Aug 20, 2018
View changes
Copy link
Contributor

@testforstephen testforstephen left a comment

How about CompletionsProvider? It has similar user errors.
...lugin/src/main/java/com/microsoft/java/debug/plugin/internal/eval/JdtEvaluationProvider.java Outdated
@@ -135,6 +137,13 @@ public void initialize(IDebugAdapterContext context, Map<String, Object> props)
compiledExpression = engine.getCompiledExpression(expression, stackframe);
}

if (compiledExpression.hasErrors()) {
completableFuture.completeExceptionally(AdapterUtils.createUserErrorDebugException(
String.format("Cannot evalution expression because of compilation error(s): %s.",

This comment has been minimized.

Sign in to view
@testforstephen

testforstephen Aug 20, 2018
Contributor

typos. Cannot evalution -> Cannot evaluate

This comment has been minimized.

Sign in to view
@andxu

andxu Aug 21, 2018
Author Collaborator

done
...re/src/main/java/com/microsoft/java/debug/core/adapter/handler/InitializeRequestHandler.java
@@ -53,6 +53,7 @@
caps.supportsCompletionsRequest = true;
caps.supportsRestartFrame = true;
caps.supportsLogPoints = true;
caps.supportsEvaluateForHovers = true;

This comment has been minimized.

Sign in to view
@testforstephen

testforstephen Aug 20, 2018
Contributor

By mistake? or Do you want to support EvaluateForHovers in this PR?

This comment has been minimized.

Sign in to view
@andxu

andxu Aug 21, 2018
Author Collaborator

No, it is expected in this pr to enable EvaluateForHovers
...core/src/main/java/com/microsoft/java/debug/core/adapter/handler/EvaluateRequestHandler.java Outdated
throw AdapterUtils.createCompletionException(
String.format("Cannot evalution expression because of %s.", cause.toString()),
String.format("Cannot evaluation expression because of %s.", cause.toString()),

This comment has been minimized.

Sign in to view
@testforstephen

testforstephen Aug 20, 2018
Contributor

typos. evaluation -> evaluate

This comment has been minimized.

Sign in to view
@andxu

andxu Aug 21, 2018
Author Collaborator

done
com.microsoft.java.debug.core/src/main/java/com/microsoft/java/debug/core/DebugException.java Outdated
* Create a debug exception with userError flag.
* @param message the error message
* @param errorCode the error code
* @param userError the boolean value of whether this exception is caused by a known user error

This comment has been minimized.

Sign in to view
@testforstephen

testforstephen Aug 20, 2018
Contributor

of -> indicates
...core/src/main/java/com/microsoft/java/debug/core/adapter/handler/EvaluateRequestHandler.java Outdated
ErrorCode.EVALUATE_FAILURE);
// stackFrameReference is null means the given thread is running
throw new CompletionException(AdapterUtils.createUserErrorDebugException(
"Failed to evaluate. Reason: Cannot evaluate because the thread is resumed.",

This comment has been minimized.

Sign in to view
@testforstephen

testforstephen Aug 20, 2018
Contributor

=> Failed to evaluate. Reason: Cannot evaluate because the thread is not suspended.
@Eskibear
Eskibear reviewed Aug 20, 2018
View changes
com.microsoft.java.debug.core/src/main/java/com/microsoft/java/debug/core/DebugException.java
@@ -15,6 +15,8 @@
private static final long serialVersionUID = 1L;
private int errorCode;

private boolean userError = false;

This comment has been minimized.

Sign in to view
@Eskibear

Eskibear Aug 20, 2018
Member

confusing name userError for a boolean

This comment has been minimized.

Sign in to view
@andxu

andxu Aug 21, 2018
Author Collaborator

No better name, I think the java doc may eliminate the confusion
@Eskibear
Eskibear reviewed Aug 20, 2018
View changes
com.microsoft.java.debug.core/src/main/java/com/microsoft/java/debug/core/DebugException.java Outdated
public DebugException(String message, Throwable cause, int errorCode) {
super(message, cause);
this.errorCode = errorCode;
}


This comment has been minimized.

Sign in to view
@Eskibear

Eskibear Aug 20, 2018
Member

unnecessary newline here.
@Eskibear
Eskibear reviewed Aug 20, 2018
View changes
com.microsoft.java.debug.core/src/main/java/com/microsoft/java/debug/core/UsageDataSession.java Outdated
@@ -39,6 +40,7 @@
private Map<String, Integer> commandCountMap = new HashMap<>();
private Map<String, Integer> breakpointCountMap = new HashMap<>();
private Map<Integer, RequestEvent> requestEventMap = new HashMap<>();
private Map<String, Integer> userErrorCount = new HashMap<>();

This comment has been minimized.

Sign in to view
@Eskibear

Eskibear Aug 20, 2018 •
edited
Member

2 spaces here in private Map ?
andxu added 8 commits Aug 20, 2018
@andxu
Merge branch 'master' into andy_user_error2
Verified
This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.
GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits
Loading status checks…
638ca86
@andxu
Merge remote-tracking branch 'origin' into andy_user_error2
757f0ab
@andxu
1. fix typo
df6d078
@andxu
1. remove useless spaces.
28d0a07
@andxu
1. refine an evaluation error message
f35c9e0
@andxu
Merge branch 'andy_user_error2' of https://github.com/Microsoft/java-…
Loading status checks…
c767a60 
…debug into andy_user_error2
@andxu
1. fix checkstyle
Loading status checks…
c13d0e0
@andxu
fix checkstyle
Loading status checks…
4bbfa09
@andxu andxu force-pushed the andy_user_error2 branch from 455d2cc to 4bbfa09 Aug 21, 2018
@yaohaizh
yaohaizh reviewed Aug 21, 2018
View changes
check_style.xml
@@ -134,6 +134,8 @@
<module name="ModifierOrder"/>
<module name="EmptyLineSeparator">
<property name="allowNoEmptyLineBetweenFields" value="true"/>
<property name="allowMultipleEmptyLines" value="false"/>

This comment has been minimized.

Sign in to view
@yaohaizh

yaohaizh Aug 21, 2018

Why change this?

This comment has been minimized.

Sign in to view
@andxu

andxu Aug 21, 2018
Author Collaborator

Some bad styles are founded by yanzh but not by checkstyle, so I enhanced the rules
@andxu andxu mentioned this pull request Aug 21, 2018
Closed
@testforstephen
testforstephen reviewed Aug 21, 2018
View changes
...microsoft.java.debug.core/src/main/java/com/microsoft/java/debug/core/adapter/ErrorCode.java Outdated
COMPLETIONS_FAILURE(1017);
COMPLETIONS_FAILURE(1017),
EVALUATION_COMPILE_ERROR(2001),
EVALUATE_NOT_SUSPEND(2002);

This comment has been minimized.

Sign in to view
@testforstephen

testforstephen Aug 21, 2018
Contributor

Cannot get the failure reason from the name. -> EVALUATE_NOT_SUSPENDED_THREAD
@andxu
1. use a better error code.
Loading status checks…
b54308b
@testforstephen
testforstephen reviewed Aug 22, 2018
View changes
Copy link
Contributor

@testforstephen testforstephen left a comment

This pr looks good to me.
Please add user error for DEBUG CONSOLE Auto-Complete feature too.
andxu added 4 commits Aug 22, 2018
@andxu
Merge branch 'master' into andy_user_error2
Verified
This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.
GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits
Loading status checks…
fd015d4
@andxu
Merge remote-tracking branch 'origin' into andy_user_error2
16df313
@andxu
1.fix a checkstyle check error, this rule is newly added.
cd751f0
@andxu
Merge branch 'andy_user_error2' of https://github.com/Microsoft/java-…
Loading status checks…
49f4658 
…debug into andy_user_error2
@testforstephen
testforstephen approved these changes Aug 22, 2018
View changes
@andxu andxu merged commit e9ed6a1 into master Aug 22, 2018
3 checks passed
3 checks passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
license/cla All CLA requirements met.
Details
@andxu andxu deleted the andy_user_error2 branch Aug 22, 2018
@andxu andxu mentioned this pull request Aug 23, 2018
Closed
@testforstephen testforstephen mentioned this pull request Sep 4, 2018
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Eskibear Eskibear

@yaohaizh yaohaizh

@testforstephen testforstephen

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants
@andxu @Eskibear @testforstephen @yaohaizh
You can’t perform that action at this time.