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

Add "remote-debugging-port" switch, fixes #498. #521

Closed
wants to merge 1 commit into from

Conversation

hokein
Copy link
Contributor

@hokein hokein commented Jul 27, 2014

@zcbenz, please take a look, thanks!

@@ -89,6 +90,9 @@ void AtomBrowserMainParts::PreMainMessageLoopRun() {
static_cast<content::BrowserContext*>(AtomBrowserContext::Get())->
GetRequestContext();

devtools_delegate_.reset(
new AtomDevToolsDelegate(AtomBrowserContext::Get()));

Copy link
Member

Choose a reason for hiding this comment

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

We should only create the devtools delegate when user has specified a remote debugging port.

@zcbenz
Copy link
Member

zcbenz commented Jul 29, 2014

@hokein Can you move the code to https://github.com/brightray/brightray? Our current design is to handle all devtools related things in brightray when possible.

@hokein
Copy link
Contributor Author

hokein commented Jul 30, 2014

@zcbenz, Done.

@@ -79,6 +81,12 @@ void AtomBrowserMainParts::PreMainMessageLoopRun() {
static_cast<content::BrowserContext*>(AtomBrowserContext::Get())->
GetRequestContext();

if (CommandLine::ForCurrentProcess()->HasSwitch(
Copy link
Member

Choose a reason for hiding this comment

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

This code had better be put in brightray::BrowserMainParts.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

How about putting the devtools_delegate_ to brightray::BrowserMainParts? In this way, there is no modification on atom-shell parts.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, all the code of this feature should be put in brightray, in atom-shell we only need to update the vendor/brightray.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done. Close this issue since no need to modify the code of atom-shell.

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.

None yet

2 participants