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

Updating telemetry and some comments #1825

Merged
merged 3 commits into from Oct 4, 2019

Conversation

@karthiknadig
Copy link
Member

commented Oct 3, 2019

Fixes #1818

@karthiknadig karthiknadig requested review from fabioz and int19h as code owners Oct 3, 2019
@@ -356,7 +356,8 @@ def make_input_requested_message(self, started):
def make_skipped_step_in_because_of_filters(self, py_db, frame):
msg = 'Frame skipped from debugging during step-in.'
if py_db.get_use_libraries_filter():
msg += '\nNote: may have been skipped because of "justMyCode" option (default == true).'
msg += ('\nNote: may have been skipped because of "justMyCode" option (default == true). '
'Try setting \"justMyCode\": false in the launch.json file.')

This comment has been minimized.

Copy link
@int19h

int19h Oct 3, 2019

Contributor

launch.json is a VSCode artifact, so if we mention it here, let's phrase it somewhat more generic. Something like:

Try setting ... in debug configuration (launch.json)

This comment has been minimized.

Copy link
@fabioz

fabioz Oct 3, 2019

Collaborator

Maybe we could store the clientID on PyDB and then check the clientID to create an explicit message for VSCode/other IDEs? (creating this part of the message should be refactored to a method in this case)

This comment has been minimized.

Copy link
@karthiknadig

karthiknadig Oct 4, 2019

Author Member

For now i will make this a simple string as suggested by @int19h. We have a separate work item to be more accurate on identifying why the file was skipped. That will require getting different messages based on rules or just-my-code setting.

This comment has been minimized.

Copy link
@Anapo14

Anapo14 Oct 4, 2019

Contributor

We may need to circle back around to this. I'm thinking about the case in which a user starts a debugging session from the Debug menu option: Debug > Start Debugging. In this case, a launch.json file isn't created on their behalf. I think having this is a good first 'catch-all' for users who do manage to go through the 'Add Configuration' process.

This comment has been minimized.

Copy link
@karthiknadig

karthiknadig Oct 4, 2019

Author Member

Note that from the debugger side we really cannot distinguish between configuration from 'launch.json' or if it is created on the fly. Ideally as I have prescribed before there has to be a mechanism in DAP and VSCode to show these type of potential setting changes in a way that doesn't cause notification overload.

This comment has been minimized.

Copy link
@int19h

int19h Oct 4, 2019

Contributor

Going forward, it's likely that we'll need a custom DAP message to report a file being skipped. The client can then look at its debug configuration, and formulate the message appropriately (or even offer to remedy it directly). This would also provide better control over notifications, in accordance with the UX guidelines of the client.

@int19h
int19h approved these changes Oct 3, 2019
@karthiknadig karthiknadig merged commit ffc6306 into microsoft:master Oct 4, 2019
2 of 4 checks passed
2 of 4 checks passed
ptvsd-testing-automation #20191003.6 failed
Details
continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
SonarCloud Code Analysis Quality Gate passed
Details
license/cla All CLA requirements met.
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.