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

[wasm][debugger] Implement Runtime.evaluate. #62142

Merged
merged 1 commit into from
Nov 30, 2021

Conversation

thaystg
Copy link
Member

@thaystg thaystg commented Nov 29, 2021

Implement Runtime.Evaluate, the behavior is the same of Debugger.EvaluateOnCallFrame and use the first frame to evaluate.
Implementing it make possible to set variable value from VSCode and Visual Studio.

Fixes #61974
Original Issues: https://dev.azure.com/devdiv/DevDiv/_workitems/edit/1366427/
JSDebugger Issue: microsoft/vscode-js-debug#1143

@ghost
Copy link

ghost commented Nov 29, 2021

Tagging subscribers to this area: @thaystg
See info in area-owners.md if you want to be subscribed.

Issue Details

Implement Runtime.Evaluate, the behavior is the same of Debugger.EvaluateOnCallFrame and use the first frame to evaluate.

Fixes #61974

Author: thaystg
Assignees: -
Labels:

area-Debugger-mono

Milestone: -

@@ -329,7 +329,7 @@ internal static async Task<JObject> CompileAndRunTheExpression(string expression
expression = expression.Trim();
if (!expression.StartsWith('('))
{
expression = "(" + expression + ")";
expression = "(" + expression + "\n)";
Copy link
Member

Choose a reason for hiding this comment

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

Is this just for readability?

Copy link
Member Author

Choose a reason for hiding this comment

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

No! The code that VS tries to evaluate is like I added in the test case.
Something like this:
15 //evaluate from VS
So I added this \n to avoid to comment the ")".

Copy link
Member

Choose a reason for hiding this comment

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

can you add that as a code comment here? And maybe a \n before the expression too?

Comment on lines +406 to +411
if (context.CallStack != null)
{
Frame scope = context.CallStack.First<Frame>();
return await OnEvaluateOnCallFrame(id,
scope.Id,
args?["expression"]?.Value<string>(), token);
Copy link
Member

Choose a reason for hiding this comment

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

super-nit:

Suggested change
if (context.CallStack != null)
{
Frame scope = context.CallStack.First<Frame>();
return await OnEvaluateOnCallFrame(id,
scope.Id,
args?["expression"]?.Value<string>(), token);
if (context.CallStack?.Length > 0)
{
return await OnEvaluateOnCallFrame(id,
context.CallStack[0].Id,
args?["expression"]?.Value<string>(), token);

@@ -329,7 +329,7 @@ internal static async Task<JObject> CompileAndRunTheExpression(string expression
expression = expression.Trim();
if (!expression.StartsWith('('))
{
expression = "(" + expression + ")";
expression = "(" + expression + "\n)";
Copy link
Member

Choose a reason for hiding this comment

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

can you add that as a code comment here? And maybe a \n before the expression too?

@radical
Copy link
Member

radical commented Nov 29, 2021

btw, can you add to the PR description why this is being added, referencing the original issue, and the jsdebugger issue too?

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

Successfully merging this pull request may close these issues.

The value of the parameter cannot be changed when debugging the WASM project.
2 participants