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

Forward source requests to debugpy as fallback #875

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

vidartf
Copy link

@vidartf vidartf commented Feb 22, 2022

If the source cannot be looked up by the path argument, forward it to debugpy as it might be able to resolve it using the sourceReference field.

EDIT: If the sourceReference is non-zero, forward it to debugpy, as the specification requires the reference to be used in those cases.

Proposed fix to #870.

TODO: Write some tests.

If the source cannot be looked up by the `path` argument, forward it to debugpy as it might be able to resolve it using the sourceReference field.
reply['success'] = False
reply['message'] = 'source unavailable'
reply['body'] = {}
reply = await self._forward_message(message)
Copy link
Contributor

@JohanMabille JohanMabille Feb 23, 2022

Choose a reason for hiding this comment

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

See my comment; I think the right implementation should be to handle the request in the kernel if and only if the attribute sourceReference is 0. Notice that the source attribute is optional, according to the DAP, therefore this implementation is not totally compliant.

Copy link
Author

Choose a reason for hiding this comment

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

I've had another go at this, trying to follow the spec more closely this time.

Give `sourceReference` priority. Not immediately clear which location should be given priority if soruceReference and source.sourceReference are both defined and different values. Spec says that they should be the same, so we can claim that this is not our problem.
}
source_ref = message["arguments"].get("sourceReference", 0)
if source_ref == 0:
message["arguments"].get("source", {}).get("sourceReference", 0)
Copy link

Choose a reason for hiding this comment

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

Suggested change
message["arguments"].get("source", {}).get("sourceReference", 0)
source_ref = message["arguments"].get("source", {}).get("sourceReference", 0)

Copy link

Choose a reason for hiding this comment

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

I tested this change and it worked with and without the suggested change (as VS Code passes the sourceReference in both places, SourceRequest.sourceReference and sourceRequest.source.sourceReference)

Copy link
Author

Choose a reason for hiding this comment

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

@r3m0t Thanks for testing. This change is meant to solve this for non-vscode usages as well 👍

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

Successfully merging this pull request may close these issues.

3 participants