-
Notifications
You must be signed in to change notification settings - Fork 29.7k
Pass app.started events to the DAP client + dart.debuggerUris for Profile mode #106337
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
Conversation
This allows a DAP client to know when an app has finished launching even if there is no VM Service (such as noDebug runs). Fixes Dart-Code/Dart-Code#3945 (with some client updates).
| 'vmServiceUri': serviceUri.toString(), | ||
| }), | ||
| eventType: 'dart.debuggerUris', | ||
| ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have a CL that adds a method to the base to avoid duplicating it here, but it needs a new DDS release (and then rolling into here). Assuming that doesn't happen before this is ready, I'll sort this in a separate PR once it does.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
are you saying the DDS change hasn't been published yet?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm guessing it's (going to be) this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's in DDS 2.2.3 published today, however that can't be rolled into Flutter yet due to a conflicting vm_service constraint with DWDS.
In case it isn't clear, the DDS change is not required for this code to function, only to replace lines 412-417 with a simpler base method call so that the Flutter adapter doesn't need to construct the dart.debuggerUris message itself.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm guessing it's (going to be) this
Yup! The real change was this, which is just extracting a method so this Flutter adapter can call it:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's in DDS 2.2.3 published today, however that can't be rolled into Flutter yet due to a conflicting vm_service constraint with DWDS.
In case it isn't clear, the DDS change is not required for this code to function, only to replace lines 412-417 with a simpler base method call so that the Flutter adapter doesn't need to construct the
dart.debuggerUrismessage itself.
Understood, I was hoping I could roll forward to simplify this change, but I understand we're blocked on transitive deps.
christopherfujino
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
This allows a DAP client to know when an app has finished launching even if there is no VM Service (such as noDebug runs) and also still get the VM Service URI in the case where the adapter doesn't connect to it itself (Profile mode - where we use VM Service for DevTools in the client, but don't provide any debugging).
Fixes Dart-Code/Dart-Code#3945 (with some client updates).
Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.