Repository navigation
Uniform way to trigger debugger on first line #12630
Description
Activity
After hearing from the IDE vendors, IMHO we should forget about
--inspect-brk, it won't get adopted, and just keep--debug-brkpermanently.
Protocol detection & resolution is done in orthogonal ways anyway.I'm OK with keeping
--debug-brkpermanently as the flag meaning "break-on-first-line" (too bad it wasn't called that originally). It seems analogous (to me) with hownode debugstarts the inspector in most recent. In which case, we don't have to backport anything, we just start the deprecation process for--inspect-brk(so long, barely had time to know you).I'm also OK with backporting
--inspect-brk.It seems to me that if you ignore the
--inspect --debug-brkcombo, then the situation becomes much more simple. Going forward we want a flag that means "start the debugger and break on the first line", and we have two options. The pros and cons are:Choice Pros Cons --inspect-brk--inspect+--inspect-brkmakes senseit's another option to remember, --debug-brkalready exists--debug-brkOld flag, new protocol, just keeps working. Also more intuitive if you don't know or care what the inspector is. May be confusing that --debug-brkon v6 !==--inspect --debug-brkon v6 and !==--debug-brkon v7Thinking about this further, I'd be +1 for abandoning
--inspect-brkand sticking with--debug-brk. By Node 10 or 11, no-one will (hopefully) remember or need to care about the difference between the two protocols, we'll just have the one. And at that point having--debug-brkmean "start a debugger and break" seems like the natural choice.If we're going to do that then we should not backport
--inspect-brk, and we should re-add--debug-brkto master.EDIT: I'd also say that the number of times I've used the inspector without the
brkoption is very small, I'd say this is really the default option for a user.I'm OK with keeping --debug-brk permanently as the flag meaning "break-on-first-line" (too bad it wasn't called that originally).
@sam-github the thing is that you're not supposed to do
--inspect --debug-brk, you're just supposed to do--debug-brk, which debugs and breaks. So it actually seems pretty well named to me. The--inspect --debug-brkthing is a temporary aberration caused by having two debug protocols in one version of Node.@sam-github the thing is that you're not supposed to do --inspect --debug-brk, you're just supposed to do --debug-brk, which debugs and breaks. So it actually seems pretty well named to me. The --inspect --debug-brk thing is a temporary aberration caused by having two debug protocols in one version of Node.
If you look at the code it's actually syntactic sugar for three operations:
- Choose protocol
- Set port
- Break on first line
I think ideally it would have only been used by users, while vendors used the three explicit args
--inspect --brake-on-first --debugger-port=5599BTW: should it be "break" of "brake"?
I think it's break as in breakpoint.
- I think we should first define the best ux for debugging with a break, and then work backwards.
33 remaining items
Cross-posting from: #12580 (comment)
-1, I don't really see the point in keeping it when
--debug-brkwas going to be removed in a major.I would be for having it print out a notice to use
--inspect-brkthough.
the vendors can't/won't backport
--inspect-brkso all current IDEs will not be albe to debug node8If they can "back-port"
--inspectand detect that it is a version that needs the inspector, what is stopping them for doing the same for the similar*-brkflag?Reacted by Refael AckermannI'll try to sum:
- current versions of IDEs work with both protocols
- vendors have been exclusively using
--inspect --debug-brkto triggerinspector - if we don't restore alias current IDEs will not work with node8
- Give vendors a year to shift
the vendors can't/won't backport
--inspect-brkso all current IDEs will not be albe to debug node8If they can "back-port"
--inspectand detect that it is a version that needs the inspector, what is stopping them for doing the same for the similar*-brkflag?They all had a release cycle while both protocols were available. Unfortunately they used
--inspect --debug-brkto triggerinspector. That's hardcoded logic in all current versions.If they can "back-port" --inspect and detect that it is a version that needs the inspector, what is stopping them for doing the same for the similar *-brk flag?
To elaborate on what @refack already said: What was stopping them was that
--inspect-brkunfortunately didn't exist yet when they started to work on the integration. So they couldn't do the "right" thing.CTC agreed to restore --inspect --debug-brk as alias to --inspect-brk, pending @ChALkeR's investigating an issue.
This is my understanding as well, and #12949 accomplishes that (and more).
@jkrems
going back to --debug (or --debug-brk w/o --inspect) anytime soon is pretty evil. The same command line option combination shouldn't trigger 2 completely different protocols across different versions of node.
I agree, and in fact I think this might be @ChALkeR's primary concern.
Based on this, #12949 should not add back
--debugat all.On the other hand, it could be helpful to our users to print a friendly message when
--debugis specified in 8.x. I don't think that should be part of #12949 but would be appropriate for another PR.Reacted by Refael Ackermann and Jan Olaf Martin- added a commit that references this issue
on May 29, 2017 - added a commit that references this issue
on May 29, 2017
Ref: #12364
Have
--inspect --debug-brkas a uniform way to trigger debug on first line--inspect --debug-brkcombo to v7+v8 (inspector: restore --debug-brk alias #12580)--inspect-brk(re: inspector: makedebugan alias forinspect#11441)(IMHO if we keep it
--debug-brk,--inspect-brkwill probably never be used)node8?--debug-port/inspect-port?v6andv7requires the combo--inspect[=port] --debug-brk[=port]is specifyingporton both args a valid invocation, and in that case whichportwins?Have
--inspect-brkas a uniform way to trigger debug on first line--inspect --debug-brkcombo in v7 alone (deprecation notice yes/no) (i.e. don't land src: Remove support for --debug #12197 in v7)--inspect-brkalias tov6(inspector: enable --inspect-brk in v6 #12615)[new comment] this will make
--inspect-breaka feature of recent versions of 6.x, but it can't change the past: versions of 6.x will always exist without this feature, and so will not be debuggable by third-party tooling [without special treatment]v4it's irrelevant since it's a different protocol and other means of detection and handling is necessaryHelp the users adapt to our plan:
runtimeExecutableversion detection microsoft/vscode-node-debug2#100)User feedback
I'm trying to get more feedback from @roblourens and JetBrains, so you could make the best decision.
Quote from youtrack#WEB-26568
Comment from @roblourens regression: 3rd party debuggers are incompatible with node8 nighlies #12364 (comment)
P.S. at present WebStorm and IDEA based IDEs can't trigger debug in node8 nightlies (nor can VSCode)