Add support for address param in launch config that allows remote debugging #50
Conversation
|
Hi @nojvek, I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution!
TTYL, MSBOT; |
| @@ -162,16 +162,17 @@ export class ChromeConnection { | |||
| /** | |||
| * Attach the websocket to the first available tab in the chrome instance with the given remote debugging port number. | |||
| */ | |||
| public attach(port: number, url?: string): Promise<void> { | |||
| public attach(port: number, url?: string, address?: string): Promise<void> { | |||
There was a problem hiding this comment.
Logically I'd expect tha arguments to be : Address, port and url.
This smells of an after-thought... just one more param, right? I'd rather make sure the arguments are ordered logically, bump the sem-major version and update the two consumers of this debugger (edge + chrome)
There was a problem hiding this comment.
I thought about it. But since port is required, the other two are optional.
Having port, address, url looks equally weird as well.
On Tuesday, June 21, 2016, Kenneth Auchenberg notifications@github.com
wrote:
In src/chrome/chromeConnection.ts
#50 (comment)
:@@ -162,16 +162,17 @@ export class ChromeConnection {
/**
* Attach the websocket to the first available tab in the chrome instance with the given remote debugging port number.
*/
- public attach(port: number, url?: string): Promise {
- public attach(port: number, url?: string, address?: string): Promise {
Logically I'd expect tha arguments to be : Address, port and url.
This smells of an after-thought... just one more param, right? I'd rather
make sure the arguments are ordered logically, bump the sem-major version
and update the two consumers of this debugger (edge + chrome)—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/Microsoft/vscode-chrome-debug-core/pull/50/files/813face911502dab6c1b5d5bb3b3c8f2e2a7b9c9#r67997645,
or mute the thread
https://github.com/notifications/unsubscribe/AA-JVFpWtl7_wmkZg5pp2ywh03JzgMlGks5qOMzrgaJpZM4I7Oc5
.
Not using browser-launcher2 because it forces a bunch of CLI params that I don't want. Not using a package to detect browsers because this is straightforward. And not moving launching out of -core because it's used by the cordova extension which consumes -core, and makes sense to keep it.
No description provided.