Support Chrome SkipLists #1176
Support Chrome SkipLists #1176
Conversation
| @@ -574,7 +579,17 @@ class Debugger extends Domain { | |||
| } else { | |||
| // If we don't have source location continue stepping. | |||
| if (_isStepping && (await _sourceLocation(e)) == null) { | |||
| await _remoteDebugger.stepInto(); | |||
| var frame = e.params['callFrames'][0]; | |||
There was a problem hiding this comment.
Is params['callFrames'] an object from the webkit package? If so, can we use types here instead of map references?
There was a problem hiding this comment.
It's currently not typed unfortunately.
There was a problem hiding this comment.
But from the webkit inspection protocol lib? If so, can you open an issue to expose this?
| /// See https://chromedevtools.github.io/devtools-protocol/tot/Debugger/#method-stepInto | ||
| /// | ||
| /// Can return a cached value. | ||
| Future<List<Map<String, dynamic>>> compute( |
There was a problem hiding this comment.
What is this a list of a map of?
There was a problem hiding this comment.
A skipList is a list of LocationRanges as defined by the Chrome DevTools protocol. See the link the doc comment.
There was a problem hiding this comment.
Can you in-line that info in the dartdoc?
| for (var location in locations) { | ||
| // Convert to zero based and do not include the known location. | ||
| var endLine = location.jsLocation.line - 1; | ||
| var endColumn = location.jsLocation.column - 2; |
There was a problem hiding this comment.
Broke it out and added comments to make it clear.
| int startColumn, | ||
| int endLine, | ||
| int endColumn, | ||
| ) => |
There was a problem hiding this comment.
As a nit, I don't like to use the shorthand syntax if the method body is more than one line.
There was a problem hiding this comment.
I prefer the fat arrow because we are returning a map so the curly braces are less confusing imo. I can change it if you feel strongly though.
| @@ -48,3 +48,7 @@ dev_dependencies: | |||
|
|
|||
| executables: | |||
| webdev: | |||
|
|
|||
| dependency_overrides: | |||
There was a problem hiding this comment.
Did you mean to check this in?
There was a problem hiding this comment.
Yes. I updated the changelog so it's clear.
| @@ -574,7 +579,17 @@ class Debugger extends Domain { | |||
| } else { | |||
| // If we don't have source location continue stepping. | |||
| if (_isStepping && (await _sourceLocation(e)) == null) { | |||
| await _remoteDebugger.stepInto(); | |||
| var frame = e.params['callFrames'][0]; | |||
There was a problem hiding this comment.
But from the webkit inspection protocol lib? If so, can you open an issue to expose this?
| /// See https://chromedevtools.github.io/devtools-protocol/tot/Debugger/#method-stepInto | ||
| /// | ||
| /// Can return a cached value. | ||
| Future<List<Map<String, dynamic>>> compute( |
There was a problem hiding this comment.
Can you in-line that info in the dartdoc?
| int startColumn, | ||
| int endLine, | ||
| int endColumn, | ||
| ) => |
| @@ -48,3 +48,7 @@ dev_dependencies: | |||
|
|
|||
| executables: | |||
| webdev: | |||
|
|
|||
| dependency_overrides: | |||
This greatly reduces the number of back and forth step requests as Chrome will continue stepping until we hit a range not included in the provided
skipList. Read more about the feature here.