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

Allow hinting DebugProtocol.Source to prefer path over sourceReference #224

Open
SchoofsKelvin opened this issue Sep 15, 2021 · 4 comments
Labels
clarification Protocol clarification feature-request Request for new features or functionality

Comments

@SchoofsKelvin
Copy link

It'd be nice if a new field (e.g. preference) gets added to DebugProtocol.Source to tell clients (IDEs) that if the Source can be mapped to a file, it should open that file instead of using sourceReference. Perhaps it's even worth making this the default, especially when combined with my item of checknotes below.

Details

  • It's not uncommon to end up debugging a program for which the input source has changed.
    • As in, the debugger still knows the original input source that the program is currently running.
    • A good example is NodeJS with JavaScript, and even TypeScript if the (loaded) source maps also contain the source.
  • Currently the protocol states that a sourceReference bigger than 0 means that should be used, regardless of path.
    • This would either be a "breaking" change (although it won't break anything, IDEs just wouldn't show "modern" behavior).
    • We can specify to only prefer path if checksums is present to validate whether it results in the correct file (contents).
  • It is more user friendly in IDEs if a Source can be mapped to the actual source file, instead of a snapshot in the debugger.
    • The user might already have the file open, and focusing it instead of opening a cached file is preferred.
    • The user can now also directly edit the file, without having to first look for the correct file based on the read-only snapshot.
  • The IDE might find the right file, but maybe its contents have changed:
    • We can use checksums to validate whether it is (the correct version of) the correct file. If not, revert to sourceReference.
  • We have the sources field but this isn't (meant as) a 1-on-1 mapping of the source itself.
  • Having the debug adapter only provide path is not enough:
    • If the file isn't found, the IDE has nothing to show.
    • If the file changed (and checksums isn't present), the IDE might display an outdated (or completely wrong) file.
    • If checksums is present, the IDE can validate whether the file (content) is correct, but not fallback to anything if it isn't.

This is mostly a big quality-of-life improvement towards IDE users, as these would now prefer the actual source files over cached versions provided by the debug adapter, assuming they're the same (using checksums). While the file content will be the exact same, mapping to an editable source file or mapping to a read-only "debug content file" is quite a large difference.

Hotreload example

Since this would still allow different Sources with the same path but a different sourceReference/checksums (and the IDE automatically picking between showing the path file or the read-only sourceReference viewer), we can also easily allow debug adapters to differentiate different versions of the same (path-specified) file, e.g. because of hotreloading:

  • Program loads a.js with AAA as checksum.
  • Debugger attaches, adapter tells the IDE about { path: 'a.js', sourceReference: 1, checksum: ['AAA'] }.
    • IDE finds the properly mapped a.js, validates the checksum and opens it.
  • User edits file and saves it, causing the program/debugger to hotreload it one way or another.
    • Let's say the new content results in BBB as checksum.
  • Debug adapter tells the IDE about { path: 'a.js', sourceReference: 2, checksum: ['BBB'] }.
    • IDE finds the file again (which might still be open), sees the checksum matches the (new) content, so opens it.
  • Debug adapter might tell IDE again about the previous file (ref 1, checksum AAA).
    • IDE finds the file but notices the different checksum. Falls back to using the sourceReference with value 1.
  • Debug adapter might tell IDE again about the current file (ref 2, checksum BBB) after it was saved, but before the hotreload.
    • IDE finds the file but notices the different checksum. Falls back to using the sourceReference with value 2.

Of course, this hotreload example is an advanced use case this change would allow. Just the QoL improvement to open the true (editable) source file instead of a snapshot if possible is already a very nice use case this allows.

@weinand weinand transferred this issue from microsoft/vscode-debugadapter-node Oct 19, 2021
@weinand weinand self-assigned this Oct 19, 2021
@weinand weinand added feature-request Request for new features or functionality clarification Protocol clarification labels Oct 19, 2021
@weinand weinand removed their assignment Nov 24, 2022
@DanTup
Copy link
Contributor

DanTup commented Jan 22, 2024

I found this issue while trying to find out how checksums worked. There's not much info in the spec about it ("The checksums associated with this file.") or what the client should do.

I'm interested in something like the above.. in particular I'd like to have VS Code use sources from disk when they match what's in the runtime (because if we download from the VM, the user sees two copies of the same file and has two sets of breakpoints and it's not clear why), but use downloaded source if they don't match (although this raises some questions about breakpoints).

I can't any mention in the VS Code API docs about checksums.

@weinand is it possible to implement anything at all like this today (for example just avoiding confusion from sources on disk not matching what's in the runtime)?

@int19h
Copy link

int19h commented Jan 22, 2024

It would be nice to have it. We've hit similar issues with Python debugging, so now there's logic in place there to report "path" only even when source is actually retrievable. It would be nice to be able to report both and let the client decide smartly on which one is the most appropriate.

@fbrosseau
Copy link
Contributor

fbrosseau commented Jan 22, 2024

I also had a related perspective on this:
#189

Also related #407

@Bramwelian
Copy link

It is nice for debugging. Bring it on

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clarification Protocol clarification feature-request Request for new features or functionality
Projects
None yet
Development

No branches or pull requests

6 participants