Skip to content

"copy value" from variables debugger window not working #290

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.

Sign up for GitHub

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

Closed
wants to merge 1 commit into from
Closed

"copy value" from variables debugger window not working #290

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion ...e/src/main/java/com/microsoft/java/debug/core/adapter/handler/EvaluateRequestHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ public CompletableFuture<Response> handle(Command command, Arguments arguments,
}
long threadId = stackFrameReference.getThread().uniqueID();
if (value instanceof ObjectReference) {
VariableProxy varProxy = new VariableProxy(stackFrameReference.getThread(), "eval", value);
VariableProxy varProxy = new VariableProxy(stackFrameReference.getThread(), "eval", value, null, expression);
int indexedVariables = -1;
Value sizeValue = null;
if (value instanceof ArrayReference) {
Expand Down
2 changes: 1 addition & 1 deletion ...ore/src/main/java/com/microsoft/java/debug/core/adapter/handler/ScopesRequestHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ public CompletableFuture<Response> handle(Command command, Arguments arguments,
return CompletableFuture.completedFuture(response);
}
ThreadReference thread = stackFrameReference.getThread();
VariableProxy localScope = new VariableProxy(thread, "Local", stackFrameReference);
VariableProxy localScope = new VariableProxy(thread, "Local", stackFrameReference, null, null);
int localScopeId = context.getRecyclableIdPool().addObject(thread.uniqueID(), localScope);
scopes.add(new Types.Scope(localScope.getScope(), localScopeId, false));

Expand Down
2 changes: 1 addition & 1 deletion ...rc/main/java/com/microsoft/java/debug/core/adapter/handler/SetVariableRequestHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ public CompletableFuture<Response> handle(Command command, Arguments arguments,
if (newValue instanceof ObjectReference && VariableUtils.hasChildren(newValue, showStaticVariables)) {
long threadId = ((VariableProxy) container).getThreadId();
String scopeName = ((VariableProxy) container).getScope();
VariableProxy varProxy = new VariableProxy(((VariableProxy) container).getThread(), scopeName, newValue);
VariableProxy varProxy = new VariableProxy(((VariableProxy) container).getThread(), scopeName, newValue, (VariableProxy) container, name);
referenceId = context.getRecyclableIdPool().addObject(threadId, varProxy);
}

Expand Down
11 changes: 9 additions & 2 deletions .../src/main/java/com/microsoft/java/debug/core/adapter/handler/VariablesRequestHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -230,14 +230,21 @@ public CompletableFuture<Response> handle(Command command, Arguments arguments,
}

int referenceId = 0;

boolean containerIsArray = containerNode.getProxiedVariable() instanceof ArrayReference;
String evaluateName;

if (indexedVariables > 0 || (indexedVariables < 0 && VariableUtils.hasChildren(value, showStaticVariables))) {
VariableProxy varProxy = new VariableProxy(containerNode.getThread(), containerNode.getScope(), value);
VariableProxy varProxy = new VariableProxy(containerNode.getThread(), containerNode.getScope(), value, containerNode, javaVariable.name);
referenceId = context.getRecyclableIdPool().addObject(containerNode.getThreadId(), varProxy);
evaluateName = varProxy.getEvaluateName();
Copy link
Contributor

@testforstephen testforstephen Aug 21, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

For the Collection and Map type, we did special handling to show their logical structure, so their display style looks like array.
image

Currently when you hover on 0: HashMap$Node@64, and click Copy Value, the generated evaluateName is map.0. Evaluate engine will report Cannot evaluate because of compilation error(s): Syntax error on token ".0", delete this token..

There are two solutions in my mind.

  1. The first solution is to generate the correct evaluate name. That means we need handle the evaluate name specially for the type java.util.Map, java.util.Collection, java.util.Map$Entry.
  • If the container is java.util.Map, the evaluate name for the children is like map.entrySet().toArray()[0].
  • If the container is java.util.Map$Entry, the evaluate name for the children is like map.entrySet().toArray(new java.util.Map.Entry[0])[0].getKey().

The cons is the problem will become complex when the hierarchy is deep.

  1. The second solution is to disable Copy Value on the children of Map and Collection, only enable it on the Map and Collection itself, and return their toString value.
    For example, when hover on map, the copy operation should return {a=1}.

Similarly, when copy an array, we need return its toString() instead.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For the first solution, when the hiearchy is deep, the evaluateName is already complex, in my opinion this isn't an issue.

I really dont like the second solution.

So, if you agree, I'll implement the solution 1.

Copy link
Contributor

Choose a reason for hiding this comment

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

In solution 1, it may be a little tricky for generating the correct evaluate expression for Collection type. For example, in order to copy the nth element's children, the evaluate expression is like collection.toArray()[n].x, this will report compilation error because of java type check.

} else {
evaluateName = VariableUtils.getEvaluateName(javaVariable.name, containerNode.getEvaluateName(), containerIsArray);
}

Types.Variable typedVariables = new Types.Variable(name, variableFormatter.valueToString(value, options),
variableFormatter.typeToString(value == null ? null : value.type(), options),
referenceId, null);
referenceId, evaluateName);
typedVariables.indexedVariables = Math.max(indexedVariables, 0);
String detailsValue = null;
if (sizeValue != null) {
Expand Down
17 changes: 16 additions & 1 deletion ...bug.core/src/main/java/com/microsoft/java/debug/core/adapter/variables/VariableProxy.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,15 @@

import java.util.Objects;

import com.sun.jdi.ArrayReference;
import com.sun.jdi.ThreadReference;

public class VariableProxy {
private final ThreadReference thread;
private final String scopeName;
private Object variable;
private int hashCode;
private final String evaluateName;

/**
* Create a variable reference.
Expand All @@ -29,11 +31,20 @@ public class VariableProxy {
* the scope name
* @param variable
* the variable object
* @param container
* the variable container, if any
* @param evaluateName
* the variable evaluate name for the container context, if any
*/
public VariableProxy(ThreadReference thread, String scopeName, Object variable) {
public VariableProxy(ThreadReference thread, String scopeName, Object variable, VariableProxy container, String evaluateName) {
this.thread = thread;
this.scopeName = scopeName;
this.variable = variable;

this.evaluateName = VariableUtils.getEvaluateName(evaluateName,
container == null ? null : container.getEvaluateName(),
container != null && container.getProxiedVariable() instanceof ArrayReference);

hashCode = Objects.hash(scopeName, thread, variable);
}

Expand Down Expand Up @@ -78,4 +89,8 @@ public String getScope() {
public Object getProxiedVariable() {
return variable;
}

public String getEvaluateName() {
return evaluateName;
}
}
27 changes: 27 additions & 0 deletions ...bug.core/src/main/java/com/microsoft/java/debug/core/adapter/variables/VariableUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -250,6 +250,33 @@ public static void applyFormatterOptions(Map<String, Object> defaultOptions, boo
}
}

/**
* Get the name for evaluation of variable.
*
* @param name the variable name, if any
* @param containerName the container name, if any
* @param isArrayElement is the variable an array element?
*/
public static String getEvaluateName(String name, String containerName, boolean isArrayElement) {
if (name == null) {
return null;
}

if (isArrayElement) {
if (containerName == null) {
return null;
}

return String.format("%s[%s]", containerName, name);
}

if (containerName == null) {
return name;
}

return String.format("%s.%s", containerName, name);
}

private VariableUtils() {

}
Expand Down