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

Debug should allow uri in DebugProtocol.Source #30996

Closed
testforstephen opened this issue Jul 19, 2017 · 15 comments
Closed

Debug should allow uri in DebugProtocol.Source #30996

testforstephen opened this issue Jul 19, 2017 · 15 comments
Assignees
Labels
debug Debug viewlet, configurations, breakpoints, adapter issues feature-request Request for new features or functionality verification-needed Verification of issue is requested verified Verification succeeded

Comments

@testforstephen
Copy link

In the Java language server, we can open classfiles with source attachment in the editor. It uses virtual documents with our own uri scheme. (e.g. jdt://contents/rt.jar/java.io/PrintStream.class).

We would like the debugger's stackframe location to return that uri too.
However, current debug protocol always expects file path. See the debugSource.ts https://github.com/Microsoft/vscode/blob/master/src/vs/workbench/parts/debug/common/debugSource.ts

Ideally DebugProtocol.Source would also support a field uri that could be used instead of a path

@vscodebot vscodebot bot added the debug Debug viewlet, configurations, breakpoints, adapter issues label Jul 19, 2017
@weinand weinand added the feature-request Request for new features or functionality label Jul 19, 2017
@weinand
Copy link
Contributor

weinand commented Jul 19, 2017

The debug protocol has support for this: https://github.com/Microsoft/vscode-debugadapter-node/blob/master/protocol/src/debugProtocol.ts#L244
But VS Code uses native paths when communicating with the DA.
Flipping this switch in VS Code would probably break many debug extensions that are not prepared for passing URIs instead of native paths.
So we have to find a way around this...

@testforstephen
Copy link
Author

Currently language server protocol textDocument/definition supports uri in its response Location, and supports user customized TextDocumentContentProvider to parse the special uri scheme (e.g. jdt://contents/rt.jar/java.io/PrintStream.class).

But vscode debugger doesn't support setBreakpoints in the user customized editor. See the code debugConfigurationManager.ts#canSetBreakpointsIn. It's a problem that the user is able to view the third party source via go to definition feature, but cannot set breakpoints there.

If the debug protocol has some configuration items to allow the specific uri schemes to be set breakpoints in, it would be better.
@weinand @isidorn

@aeschli
Copy link
Contributor

aeschli commented Sep 22, 2017

@weinand What about adding a new field 'uri' to DebugProtocol.Source and deprecated 'path'.
When 'uri' is set, it will be used instead of the path. In messages to the debug adapter, both should be set for backward compatibility.

@weinand
Copy link
Contributor

weinand commented Sep 22, 2017

@aeschli before we discussing solutions to the problem, let's try to understand the problem first.

@testforstephen's comment from above says:

vscode debugger doesn't support setBreakpoints in the user customized editor.

VS Code supports setting breakpoints for "languages" that are registered for breakpoints (this is the "breakpoints" contributions point).

Has this been done for the "class" files?

If not, you can easily enable setting breakpoints everywhere via the "debug.allowBreakpointsEverywhere" setting.

If breakpoints have been registered for "class" and you still cannot set breakpoints, then this is a bug.

@aeschli
Copy link
Contributor

aeschli commented Sep 22, 2017

java-source-attach
The gif shows the problem.

  • when going to a declaration that is inside a JAR or has a source attachment, the Java language server returns a custom URI for a virtual resource: jdt://contents/rt.jar/java.io/PrintStream.class.
  • the Java extension registers a resource provider for these URIs. The content is provided on demand by the language server
  • .class files are registered to the java language mode. The debug adapter enables breakpoint for java, but setting breakpoint doesn't seem to work
  • when debugging and stepping inside a class file, the debug adapter would like to return the same URI: jdt://contents/rt.jar/java.io/PrintStream.class so that the debug UI can open that resource. But source.path is limited to file paths, so that doesn't work.
  • Currently, the debug adapter returns class file contents through the inMemory mechanism, which leads to two different editors for the same content. Also, setting breakpoints isn't possible ahead of debugging.

@weinand
Copy link
Contributor

weinand commented Sep 22, 2017

@aeschli if setting breakpoints still doesn't work, then this is a bug. @isidorn could you please investigate.

The other issue is already being solved as we speak. The DA will be able to pass jdt://contents/rt.jar/java.io/PrintStream.class as a Source.path and VS Code will not assume that it is an native path but treat it as a uri.

@isidorn
Copy link
Contributor

isidorn commented Sep 22, 2017

Thanks for pointing this out.
There was an issue in the debug code that it allowed breakpoints only for certain schemas which was bogus. This was orignally introduced to prevetn setting breakpoints in settings files, but now I simple disable setting brekapoints in .json always. Or at least until a json debugger comes along hehe.
With the vscode insiders from monday you shuold be able to nicely set breakpoints in all your .class files

@isidorn
Copy link
Contributor

isidorn commented Sep 22, 2017

This is now nicely supported also via 22d7830

Anyways you should be able to send a string uri in Source.path. And it will get properly handled here https://github.com/Microsoft/vscode/blob/master/src/vs/workbench/parts/debug/common/debugSource.ts#L33

@testforstephen let us know if it nicely works for you

@isidorn isidorn closed this as completed Sep 22, 2017
@isidorn isidorn added this to the September 2017 milestone Sep 22, 2017
@testforstephen
Copy link
Author

@isidorn @weinand @aeschli thank you for the discussions. The solution looks perfect for me, i can't wait to verify it in the coming insider version. I'll update the status after i verified it in the next insider version.

@testforstephen
Copy link
Author

@isidorn it seems the latest insider version is published at 2017-09-20. Can you share how often the insider build is taken?

@weinand
Copy link
Contributor

weinand commented Sep 25, 2017

@testforstephen normally, Insiders is created in the morning of every working day. However, due to the hurricane activities the signing service for Windows and macOS builds is currently failing and a backup service is overloaded. You can try the linux builds.

@isidorn isidorn added the verification-needed Verification of issue is requested label Sep 25, 2017
@weinand
Copy link
Contributor

weinand commented Sep 25, 2017

@aeschli It would be great, if we could verify this with the Java extension.

@testforstephen
Copy link
Author

@weinand @aeschli @isidorn Today i have tried the latest insider in ubuntu 14.04 LTS, the feature works like a charm. Great thanks for the work.

Now for third-party class, the java debugger just need send the uri in Source.path, VSCode will use jdt content provider (registered by vscode-java extension) to render the class source automatically. Besides, i can also add breakpoint in jdt content provider before launching debugger. The debugger could work with java language server extension seamlessly.

Below is the insider version i used.
image

@isidorn isidorn added the verified Verification succeeded label Sep 28, 2017
@testforstephen
Copy link
Author

@weinand @isidorn
Log one more little thing here.

When launching a new debug session, the uri paths of the previously persisted breakpoints in the jdt content provider seem not consistent with the original uri that the java debugger returns.

The original uri in the StackTrace response body returned by the java debugger is jdt://contents/rt.jar/java.io/PrintStream.class?=1.helloworld/%5C/usr%5C/lib%5C/jvm%5C/java-8-oracle%5C/jre%5C/lib%5C/rt.jar%3Cjava.io(PrintStream.class.

The uri in the breakpoint request is changed to jdt://contents/rt.jar/java.io/PrintStream.class?%3D1.helloworld%2F%5C%2Fusr%5C%2Flib%5C%2Fjvm%5C%2Fjava-8-oracle%5C%2Fjre%5C%2Flib%5C%2Frt.jar%3Cjava.io%28PrintStream.class.

It's query segment is encoded by VSCode (reference uri.toString() https://github.com/Microsoft/vscode/blob/master/src/vs/base/common/uri.ts)

Although the encoded uri can be parsed by our java debugger very well, no feature break yet.
But if VSCode can keep the uri consistent, that would be better.

@isidorn
Copy link
Contributor

isidorn commented Sep 28, 2017

@weinand introduced some encoding here due to #23194

We could potentially decode before we send it back to the adapter.

@vscodebot vscodebot bot locked and limited conversation to collaborators Nov 17, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
debug Debug viewlet, configurations, breakpoints, adapter issues feature-request Request for new features or functionality verification-needed Verification of issue is requested verified Verification succeeded
Projects
None yet
Development

No branches or pull requests

4 participants