Skip to content

Debug test lens #3539

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

Closed
hdevalke opened this issue Mar 9, 2020 · 7 comments
Closed

Debug test lens #3539

hdevalke opened this issue Mar 9, 2020 · 7 comments

Comments

@hdevalke
Copy link
Contributor

hdevalke commented Mar 9, 2020

I am the maintainer of the Rust Test Lens extension in vscode.

Issue 17 was created because it was not possible to debug crates that contain a main.rs and a lib.rs in the same project. I cannot solve that problem without parsing the rust code.

As a proof of concept I tried to implement this in rust-analyzer see: hdevalke@2e2ad8c.

Do you think it makes sense to integrate this in rust-analyzer, knowing that currently it depends on the vscode-lldb extension?

@Veetaha
Copy link
Contributor

Veetaha commented Mar 9, 2020

Hi! Thank you for the awesome extension! It does help a lot to debug rust-analyzer tests itself!

I personally don't see any impediments to merge the PoC, since rust-analyzer is not likely to replace lldb extension and is rather to be used in combination with it.
We had some discussion on this topic with @matklad at this zulip thread.

@matklad
Copy link
Member

matklad commented Mar 10, 2020

Yup, we should just do that, thanks for exploring this @hdevalke!

Although, impl-wise, I think we should try to avoid providing separate debug/run commands, and instead have a single command which can be both debugged and runed. Ie, instead of introducing DebugConfiguration, I would rather tweak Runnable two contain both the info required for run, and the info required for debug.

@matklad
Copy link
Member

matklad commented Mar 10, 2020

We also should upstream this to the LSP/VS Code. There should be a proper protocol request for telling which things can be run from a given context....

@matklad
Copy link
Member

matklad commented Mar 10, 2020

Opened microsoft/language-server-protocol#944 upstream

@hdevalke
Copy link
Contributor Author

Hi @matklad,

You are right sharing a common structure for debugging or running should work as well.
I did split up the args into args and extra_args because that's what the vscode-lldb debug configuration expects it and I did not had to search for -- in the args vector.

The only difference between runnable and debug configuration is:
for non bin targets: add --no-run, for bin target replace run with build.

I made a new commit using only the modified Runnable struct: 0714a06...hdevalke:debug-lens.

To make this user friendly there is probably some need to add vscode-lldb as an extension dependency, or show a popup to propose to install vscode-lldb if debug.startDebugging returned false.

@matklad
Copy link
Member

matklad commented Mar 11, 2020

yes, it's fine to add vscode-lldb as a dependency of rust-analyzer

bors bot added a commit that referenced this issue Mar 13, 2020
3561: feat: add debug code lens r=matklad a=hdevalke

Refs #3539

3577: Protect against infinite macro expansion in def collector r=edwin0cheng a=flodiebold

Something I noticed while trying to make macro expansion more resilient against errors.

There was a test for this, but it wasn't actually working because the first recursive expansion failed. (The comma...)

Even with this limit, that test (when fixed) still takes some time to pass because of the exponential growth of the expansions, so I disabled it and added a different one without growth.

CC @edwin0cheng 

Co-authored-by: Hannes De Valkeneer <[email protected]>
Co-authored-by: hdevalke <[email protected]>
Co-authored-by: Florian Diebold <[email protected]>
@hdevalke
Copy link
Contributor Author

@matklad @Veetaha thanks for helping me to add this functionality to rust-analyzer. It has been a pleasure to work with you.

JoshMcguigan pushed a commit to JoshMcguigan/rust-analyzer that referenced this issue Apr 1, 2020
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

No branches or pull requests

3 participants