-
Notifications
You must be signed in to change notification settings - Fork 30k
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
src: make debug_options getters public #30494
Conversation
This has me scratching my head a lot…
I think this is the main issue here – we should get Electron away from having to do this manually, and provide an API for that, rather than exposing various internals. In #30467, inspector setup would be moved into
I’m a bit confused by this – |
@addaleax since |
@codebytere Going by that, it’s probably the simplest solution if you only include the But yeah, you can feel free to go ahead and land this – just maybe update the commit message so it doesn’t give the impression that something’s being made part of the public API? |
Will do! thanks :) |
ded7f71
to
f36dd54
Compare
f36dd54
to
83370eb
Compare
This simplifies requires for those using DebugOptions, since debug_options was defined in src/node_options-inl.h and thus embedders would need to require an extra file to do what could trivially be consolidated into one. PR-URL: #30494 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: David Carlier <[email protected]>
Landed in ccdd6ef |
This simplifies requires for those using DebugOptions, since debug_options was defined in src/node_options-inl.h and thus embedders would need to require an extra file to do what could trivially be consolidated into one. PR-URL: #30494 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: David Carlier <[email protected]>
This simplifies requires for those using DebugOptions, since debug_options was defined in src/node_options-inl.h and thus embedders would need to require an extra file to do what could trivially be consolidated into one. PR-URL: #30494 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: David Carlier <[email protected]>
This simplifies requires for those using DebugOptions, since debug_options was defined in src/node_options-inl.h and thus embedders would need to require an extra file to do what could trivially be consolidated into one. PR-URL: #30494 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: David Carlier <[email protected]>
Refs #30466.
Now that Electron is parsing and passing CLI options, we can set DebugOptions ourselves through
ParseGlobalArgs
. However, we also need to create and start the inspector agent with theDebugOptions
set by users, since the inspector agentStart
function takes a parameterconst DebugOptions& options
.The most straightforward way to do this is for us to pass
env->options()->debug_options()
to our inspector agentStart
call, but sincedebug_options
was defined insrc/node_options-inl.h
we'd need to require extra files to do what could trivially be consolidated into one. This PR therefore moves those definitions into a public-facing file, allowing us to pass correct options to the inspector without needing to parse all-new ones we'd previously parsed on startup.cc @joyeecheung @addaleax
Checklist
make -j4 test
(UNIX), orvcbuild test
(Windows) passes