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

#10027 Support EvaluatableExpressions #11484

Merged
merged 1 commit into from
Aug 4, 2022

Conversation

ndoschek
Copy link
Contributor

@ndoschek ndoschek commented Jul 29, 2022

What it does

  • Implement support for plugins providing evalutable epressions
  • Update the debug hover widget to consume evaluatable expressions from the registered providers. Keep the former implementation of guessing the expression from the current line as fallback.

Contributed on behalf of STMicroelectronics

Signed-off-by: Nina Doschek ndoschek@eclipsesource.com

Fixes #10027

How to test

To test, I created a simple evalutable expression provider for the java language, available via the custom vscode extension: evaluatable-expression-extension-10027-0.0.1.zip

  1. Install the extensions Language Support for Java(TM) by Red Hat (identifier: redhat.java) and Debugger for Java (identifier: vscjava.vscode-java-debug) from the Open VSX Registry (Extensions view) in your Theia test workspace.
    On startup, it will show an information message that the provider was registered.
  2. Download and install the test extension evaluatable-expression-extension-10027.vsix in your Theia test workspace (via Extensions view - top menu bar More actions... - Install from VSIX).
  3. Create a simple Java application to debug
Simple Test application
package myproject;

public class Test {

    public static void main(String[] args) {
        for (int i = 0; i < 2; i++) {
            String logMessage = "logging a message for i=" + i;
            System.out.println(logMessage);
        }
    }
}
  1. Set a breakpoint in line 8 (sysout) and launch the file debugging
Launch config
 {
   "type": "java",
   "name": "Launch Current File",
   "request": "launch",
   "mainClass": "${file}"
 }
  1. Once the breakpoint is hit, hover over the local variable logMessage, the extension shows an information message with the cursor position and the provided word range of the expression.
    e.g. [TestEvaluatableExpressionExtension] provide word range for position { "line": 6, "character": 26 } > [ { "line": 6, "character": 19 }, { "line": 6, "character": 29 } ]

Review checklist

Reminder for reviewers

Copy link
Contributor

@colin-grant-work colin-grant-work left a comment

Choose a reason for hiding this comment

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

I believe the implementation is correct, but while testing, I discovered a problem with the implementation of our DebugHoverWidget. I tested this PR using this plugin and this debugger with the InlineValueProvider code removed to allow it to activate.

As far as I can tell, all of the EvaluatableExpression code ran as expected - the plugin code was called, the returns were correct - but the DebugHoverWidget never appeared. It seems that when this code runs

async evaluate(expression: string): Promise<ExpressionItem | DebugVariable | undefined> {
const evaluated = await this.doEvaluate(expression);
const elements = evaluated && await evaluated.getElements();
this._expression = evaluated;
this.elements = elements ? [...elements] : [];
this.fireDidChange();
return evaluated;
}

elements is undefined, and so there is nothing to render when the DebugHoverWidget requests elements

getElements(): IterableIterator<TreeElement> {
return this.elements[Symbol.iterator]();
}

and so the DebugHoverWidget does not appear. In VSCode, by contrast, the widget does appear and renders the text as returned by the debugger.

packages/debug/src/browser/editor/debug-hover-widget.ts Outdated Show resolved Hide resolved
packages/debug/src/browser/editor/debug-hover-widget.ts Outdated Show resolved Hide resolved
@JonasHelming JonasHelming added the vscode issues related to VSCode compatibility label Aug 1, 2022
@ndoschek
Copy link
Contributor Author

ndoschek commented Aug 1, 2022

Thanks for your comments @colin-grant-work!
Regarding the minor ones, I updated my commit accordingly.

About the mock debug extension, I have a more general problem, that it does not work as properly as in VSCode for me, i.e. it does not hit breakpoints and a debug session cannot be exited/terminated. It is stuck on the last line of any markdown file after hitting Continue and does not respond to the Stop button. Stepping however works, and in that case I can confirm your observation that the debug widget does not appear.
I used the current master of the mock debug extension, and omitted the provideInlineValues command as you described.
Is there something I'm missing?

As I mentioned in the description, for the Java case, the debugging session and the hover widget are working fine for me.

@colin-grant-work
Copy link
Contributor

@ndoschek, I see the problems you mention - I'm not sure what's going on there, and it should be investigated elsewhere - but it looks like it's still possible to step though, so it's possible to stop on any line you'd like, even without a breakpoint. The inability to exit the session is a bummer. I tried moving back to commit 553fbf61c7d42d9c77ead3fdcb8bc134fef11677 (and removing the InlineValueProvider code), and the same problem with breakpoints is present, but the session at least doesn't seem to crash and be unkillable at the end of the file.

@ndoschek
Copy link
Contributor Author

ndoschek commented Aug 2, 2022

Thanks @colin-grant-work, that's way better now, I'll have another look at the mentioned issue!

@colin-grant-work
Copy link
Contributor

colin-grant-work commented Aug 2, 2022

@ndoschek, I definitely think that the problems with the hover widget ought to be fixed, and the problems with the mock debug extension investigated, but in principle both are separate from the work you've done. If you'd prefer to pursue the fix for the hover widget in a separate PR, I think the plugin side of this PR is correct and complete and could be merged.

- Implement support for plugins providing evalutable epressions
- Update the debug hover widget to consume evaluatable expressions from the registered providers. Keep the former implementation of guessing the expression from the current line as fallback.

Contributed on behalf of STMicroelectronics

Signed-off-by: Nina Doschek <ndoschek@eclipsesource.com>

Fixes eclipse-theia#10027
@ndoschek
Copy link
Contributor Author

ndoschek commented Aug 3, 2022

Hi @colin-grant-work,
Thanks for your input, I had another look at the causes of the issues with the mock debug extension, and it is not yet fully clear to me what the actual cause is. Hence, I would prefer to separate those two concerns, as the support of EvaluatableExpressions via plugins is available now with this PR.
I created a separate issue for the formerly discussed issue here: #11500

Copy link
Contributor

@colin-grant-work colin-grant-work left a comment

Choose a reason for hiding this comment

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

The implementation of the EvaluatableExpressionProvider API is complete and correct. 👍

@JonasHelming JonasHelming merged commit ae5c8e8 into eclipse-theia:master Aug 4, 2022
@ndoschek ndoschek deleted the issues/10027 branch August 4, 2022 06:25
@JonasHelming JonasHelming added this to the 1.29.0 milestone Aug 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
vscode issues related to VSCode compatibility
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support EvaluatableExpressions
3 participants