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

Notebook debugging/"run by line" #126431

Closed
5 of 7 tasks
roblourens opened this issue Jun 16, 2021 · 38 comments
Closed
5 of 7 tasks

Notebook debugging/"run by line" #126431

roblourens opened this issue Jun 16, 2021 · 38 comments
Assignees
Labels
feature-request Request for new features or functionality notebook on-testplan

Comments

@roblourens
Copy link
Member

roblourens commented Jun 16, 2021

The current plan is to support both real debugging and "run by line" in Jupyter. "Run by line" is a simple debug mode that only allows stepping through the code of a single cell. It will be backed by a real debug session but with some debug UI in VS Code disabled.

Basic debugging questions/tasks

  • Enable cell breakpoint margin in a notebook
    • Show an explicit button to show the breakpoint margin. The debugger contribution could say that it supports notebooks of some type (similar to the languages property on the debugger contribution), which enables this action.
    • I need to be able to set breakpoints before I run a cell, and I don't want to show the breakpoint margin all of the time, so we need this mode.
      image
  • How does a debug session start?
    • No launch config
    • We have discussed two concepts: starting a long running debug session, or a "run and debug" button that starts a debug session just for the time a cell is being debugged. Sounds like people generally prefer the second.
    • However, we need to have this "show breakpoint margin" mode, and we could also just combine these into a single "debugging enabled" mode which is pretty much how jupyter lab works. With this enabled, the normal run button hits breakpoints. I think it's more confusing to have a separate button to "run and debug". If I set a breakpoint and want to run the cell, why do I have to go find a different run button? Do I also need "run and debug all" or "run and debug cells below", etc.?
    • I think that ideally, VS Code would manage the experience to start debugging, and would find and invoke the proper DA, but for now, we should have the extension do all of this until we figure out what the experience should actually look like, which would drive vscode/api changes.
    • The extension also can dynamically check whether the kernel supports debugging. If VS Code owns the start debugging experience, this needs to be surfaced.
      • The controller could set a debugAdapter property which tells us the ID of a debug adapter that can debug the active kernel. If it's undefined, debugging is not enabled.
  • The work to implement the debug adapter, talk to ipykernel, and jupyter-specific things Native Notebooks - Run by Line vscode-jupyter#5607
  • Restore breakpoints to correct cell when URIs change after closing/reopening notebook
    • The prototype repo has a fix for this in the extension, but vscode should do this automatically for all notebooks, maybe by saving the cell index with the breakpoint itself, or just by having some notebook contrib that moves them around.
  • Is it bad that the timer runs when paused?
    • Can pause it, but the start/end times in the API will still reflect the paused time. We could just hide this and not persist the duration when pausing in a cell.
  • [ ] Variables pane vs debug variables view
  • [ ] The debug console vs interactive pane
  • [ ] Figure out how debugging across different kernel types would work
    • If dotnet interactive and julia implemented DAP with the same extensions as Jupyter, the same debug adapter could be used - https://jupyter-client.readthedocs.io/en/stable/messaging.html#additions-to-the-dap
    • But then where does the debug adapter extension live, and how does it acquire the connection to the kernel?
    • It is probably easier to have separate debug adapters that live next to the extension code that has a connection to the kernel, we can share code with a library. However it was mentioned that this same problem basically exists with the "restart kernel" command which is pretty much the same across different kernel types but can't be contributed by a single extension without a way to get the kernel connection for other controllers.
    • We are focusing on python and the kernels provided by the vscode-jupyter extension
  • [ ] Should we put cells in readonly mode when paused?

Run by line tasks

  • Start a debug session when the "run by line" button is clicked
    • The extension will do this with vscode.debug.startDebugging
  • Ability to start a debug session in "simple mode" which "run by line" will enable. In "simple mode", the window statusbar will not change color, the debug widget will not appear, and the debug viewlet will not be shown. Everything else can remain the same.
    • How do we start a debug session in "simple mode"? We can make it part of the debug adapter's "capabilities", or a launch config prop read by vscode, or a flag on DebugSessionOptions. I think the first is preferable, but will there be debug UI shown before we get the capabilities?
    • My thinking is that "run by line" will not be a feature that all notebooks get out of the box, but something that an extension can implement using the "simple mode" flag, toolbar contributions, and other basic vscode API, because I am not sure that every notebook type in the world would agree on the details.

https://excalidraw.com/#json=5721711802580992,GeAhnXz10-VKb53A9XmgNA

cc @isidorn @weinand

@roblourens roblourens added feature-request Request for new features or functionality notebook labels Jun 16, 2021
@roblourens roblourens added this to the June 2021 milestone Jun 16, 2021
@roblourens roblourens self-assigned this Jun 16, 2021
@weinand
Copy link
Contributor

weinand commented Jun 16, 2021

@roblourens @DavidKutu some comments:

  • Breakpoints can be created and edited independent from debug sessions and the typical flow is to add breakpoints to a source file and then start debugging. Since notebooks cells do not show the breakpoint margin by default, I've introduced a "debugging mode" for notebooks. With a toggle button in the editor's header, debugging can be enabled and the breakpoint margin (and all persisted breakpoints) become visible. The breakpoint margin is controlled via the cell's metadata breakpointMargin property. Since "breakpoints" are not part of the simple "Run by line" mode, the toggle button could be repurposed as a toggle for switching between "simple" and "full" debugging mode.
  • when notebook cells are rearranged in a notebook editor, persisted cell breakpoints become outdated because their cell URIs still contain the old cell indexes. This code tries to fix this by updating the indexes in the breakpoint URIs. This code is independent from debugging and should really live close to the code which rearranges notebook cells. This problem would not exist if cell URIs would not use the cell's index but some robust ID.

BTW, my Nodebook sample shows how to implement notebook debugging functionality based on existing VS Code debugger extensions. It is simpler than the vscode-simple-jupyter-notebook because it does not rely on the DAP implementation of the xeus kernel and therefore has no DAP forwarding.

@roblourens
Copy link
Member Author

The breakpointMargin metadata is gone and we don't plan to add another way for extensions to control this. Talked more about this today, and we will probably just need some explicit switch like you previously had (or, just show the breakpoint margin all the time).

I don't think we plan to give cells a real ID, but I agree with your points about that problem.

@DonJayamanne
Copy link
Contributor

DonJayamanne commented Jun 19, 2021

igure out how debugging across different kernel types would work

I don't think .NET & Julia support debugging in Jupyter today, hence if debugging were to be implemented, i don't think it would be implemented through the jupyter protocol, as both have their corresponding VS Code extensions that support notebooks and do not go through Jupyter (i.e Jupyter protocol isn't involved in execution today in .NET/Julia/R code execution).

@davidanthoff /cc

@davidanthoff
Copy link
Contributor

Yes, that is correct for Julia: there is no debugging support in the IJulia kernel that is used by JupyterLab. But we do have a DAP implementation in the Julia VS Code extension, so my hope would be that we can just use that for the Jupyter notebook scenario that uses the Julia extension to run code.

I think my preference would be to try to get the full debugger working for Julia inside Jupyter notebooks, rather than spend time on a simplified "run by line" feature. There really wouldn't be any reason to use the latter if we have a full DAP/proper debugger experience implemented, right?

@rchiodo
Copy link
Contributor

rchiodo commented Jun 21, 2021

@davidanthoff

I think my preference would be to try to get the full debugger working for Julia inside Jupyter notebooks, rather than spend time on a simplified "run by line" feature. There really wouldn't be any reason to use the latter if we have a full DAP/proper debugger experience implemented, right?

Run by line was created first because of the feedback we got from data scientists. They didn't want a debugger, didn't see the reason for one and were not interested in using it. They just did printf debugging, why would they need something else? Run by line was our kind of debugger lite scenario so that these users would feel more comfortable with it. Right now we have about 40,000 people using it per month. I'm sure some of them would prefer full debugging, but we think run by line removes the worries about having to 'learn a debugger'.

@davidanthoff
Copy link
Contributor

Maybe the question of UI and underlying implementation is distinct? I could imagine something where the button "run line by line" just starts the debugger? And maybe there is some UI for "next line" that is really hooked to "run next statement"? It just feels that one should easily be able to implement a line-by-line UI feature if there is an existing DAP implementation for a language, without adding another API/execution model or something like that.

Also, just out of curiosity: this feature really would have to be "run statement by statement", right? Is there some API to detect the extend of the next statement? At least in Julia executing line-by-line doesn't make that much sense because there are many multi-line statements that would just be syntax errors if one tried to execute them line-by-line. We do have the feature to detect the next syntactically complete statement in our language server, but it is a custom protocol extension and we would need some more API to surface that to somewhere else, right?

@DonJayamanne
Copy link
Contributor

DonJayamanne commented Jun 21, 2021

feature really would have to be "run statement by statement", right?

Yes that's right. That's what it is today.

is a custom protocol extension and we would need some more API to

No it isn't. Is built on top of the existing dap.

feels that one should easily be able to implement a line-by-line UI feature if there is an existing DAP implementation for a language, without adding another API/execution model or something like that.

Yes technically, that's what how we implemented this for python in our webview based notebook.

@rchiodo
Copy link
Contributor

rchiodo commented Jun 21, 2021

Run by line currently does a step into on every 'F10' by the user. It then checks the frame when the stop event comes in to see where the step into landed. If in the same cell, it stops the UI. If not in the same cell, it executes a step out command. This is repeated for every step by the user until it lands outside of the current cell (or meaning the frame that called the cell to begin with)

@davidanthoff
Copy link
Contributor

@DonJayamanne Ah, cool! So essentially from our end, we "just" need to get the DAP story going with the notebooks :) I like that, because we of course want to do that in any case.

@rchiodo Ah, so it actually will descend into a nested function calls step-by-step as long as those functions are all defined in one cell? What about things like higher order functions? Say you have this Julia code:

function foo(i)
  return i*2
end

x = map(foo, sourceArray)

If you run a step-into command on the line with map, you'll stop inside the map implementation in Julia base, and I think by the logic you describe it would then just do a step-out right away and never break inside foo, right? Whereas if someone called foo directly from the global scope, in the logic you describe it would break in foo, right? Similar, if a user were to call any intermediate function that was defined in a different cell that would then call foo, it would not stop there, right? Maybe another way to implement this feature would be to set breakpoints under the hood on every line in the cell? That way one would get a really consistent experience where "run line-by-line" in a cell really stops on every line, no matter what.

I think I'm still wondering, though, whether all of this just primarily introduces more choices, and more special cases, and whether one couldn't just make the normal debug experience "non scary"? But I think I should probably just try out the existing line-by-line to get a sense how that works :)

@rchiodo
Copy link
Contributor

rchiodo commented Jun 21, 2021

For your map case (at least with the debugger for python code), it won't stop in map cause that's considered 'system' code. There's a 'just my code' feature of the DAP that prevents stepping through stuff that isn't your own code.

But yes it doesn't work for cells that jump through intermediate functions that you defined that are not in the same cell. There's a lot of cases where Run by line doesn't really 'work'. We argued about a lot of them :) The behavior we have right now is where it ended up.

There's also talk of run by line auto running to where the cursor is. Cause that's where the user is currently investigating. Why make them step all the way through the code above the cursor.

@rchiodo
Copy link
Contributor

rchiodo commented Jun 21, 2021

@davidanthoff is Julia thinking about supporting debugging in JupyterLab? We thought if kernels supported debugging there, they'd want to use the same code in VS code to light up debugging. (meaning the kernel would expose the DAP instead of an extension)

@davidanthoff
Copy link
Contributor

There are no concrete plans to add debugging to JupyterLab for Julia that I'm aware of. I think if someone wanted to do that, they would essentially reuse the DAP implementation that we have for the Julia VS Code extension and integrate that into IJulia.jl. The two peole in Julialand that are most familiar with this DAP stuff are @pfitzseb and myself, and at least I am focused on the Julia VS Code extension and getting the Jupyter story to work there, without a dependency on kernelspec installations :) My suspicion is that the same is true for @pfitzseb, given that he is also a core dev of the Julia VS Code extension.

So I think for the Jupyter notebook story inside VS Code ideally we would shoot for a debugger implementation that really only needs VS Code, Julia extension and Jupyter extension and Julia itself installed, with no other setup required and no other config files like kernelspecs needed.

@rchiodo
Copy link
Contributor

rchiodo commented Jun 21, 2021

So I think for the Jupyter notebook story inside VS Code ideally we would shoot for a debugger implementation that really only needs VS Code, Julia extension and Jupyter extension and Julia itself installed, with no other setup required and no other config files like kernelspecs needed.

@davidanthoff I think this is also possible if you were to give us an API that we could send messages to. We don't have to start a kernel (or need a kernelspec.json) in order to do 'jupyter' type stuff.

I believe @claudiaregio has sent you a document describing how we want to enable all the stuff in the jupyter extension UI for all kernels (or notebook controllers as VS code calls them). To do so, we don't need a kernel on disk, we just need some object we can call requestExecute, or requestDebug on.

This would mean you wouldn't have to duplicate the work we're doing to get debugging to work (it's not a lot but you can see some of it here: https://github.com/microsoft/vscode-jupyter/commits/david/debuging).

Or the alternative is that all notebook controllers have to reimplement the work that David has done (with the benefit of not having to write a method called 'requestDebug')

@roblourens
Copy link
Member Author

roblourens commented Aug 4, 2021

I still haven't decided on what the actual UX model for full debugging would look like. There are basically three options that have been proposed/discussed

  1. Special "Debug cell" commands which are separate from "Run cell"
  • In this model, there is a new command that runs the cell with debugging enabled. This would be exposed with a new button and keyboard shortcut.
  • Pros
    • You can have breakpoints set but still run normally without hitting them, and without having to disable the breakpoints
    • Feels similar to other things in VS Code. All Python files have a "run" button that does not debug. Separately, you can also debug.
  • Cons
    • We need to add a new button. Where does it go? If it's under the consolidated run button (chevron dropdown) then you have to click twice to use it.
    • Do we duplicate every "run" command? "Debug all", "Debug above/below", "Debug and insert", etc
    • We need to come up with new keyboard shortcuts
  1. Global debug switch (the current experience)
  • In this model, the user toggles a switch or checks a checkbox at the notebook level which enables debugging for the notebook. When the switch is on, running a cell will hit breakpoints.
  • Pros
    • No need to create new commands
    • It's clear whether debugging is enabled
    • Matches JupyterLab
  • Cons
    • Doesn't feel like the right model when I am already attached to a live kernel that is running code. Feels like I am "attaching a debugger" as a separate thing.
    • Need to invent new UX in vscode to show this toggle
  1. Debug when running automatically
  • We could make the debug session totally invisible by simply hitting breakpoints if you run a cell that has breakpoints in it
  • Pros
    • No need to click a special button or wonder why your breakpoints aren't hit when you run a cell
    • No need to create new commands
    • "Just works". I can open a notebook, set a breakpoint, and hit it
  • Cons
    • If I want to run without hitting breakpoints, I have to realize that I can disable breakpoints invidually or find the "disable breakpoints" button in the debug viewlet
      • We could solve this with a new checkbox to disable breakpoints in the notebook.
    • It's not clear how "break on exceptions" works, can I break on an exception when there is no breakpoint set?
    • What if I set a breakpoint on a .py file called by some cell, and run that cell, how do we know to enable the debugger when the cell/notebook itself has no breakpoints?

My personal preference is 3 for being the easiest to use. 1 has also been popularly suggested, but I am not crazy about extra buttons/shortcuts, or about having to remember to do something different, when keyboard shortcuts for running cells are baked into a pro user's muscle memory. 2 might actually be a compromise, but I don't think anyone was excited about it.


Then there is the question of UX for run-by-line. Currently this shows up in a button in the cell toolbar. We could:

  1. Leave it in the cell toolbar all the time
  • Feels like a mismatch with the run button which is on the left side, and the cell toolbar is on the right by default
  1. Show it in the consolidated run button, (so you have to click twice to start a RBL session) and show it in the cell toolbar while running for stepping with a single click.
  • This works well with option 1 for full debugging if we put extra debug commands under the consolidated run button
  • But it may be unclear that you have to find the new button in the cell toolbar to step

@weinand
Copy link
Contributor

weinand commented Aug 4, 2021

@roblourens I like option 3 too and I assume that you would run with debugging enabled only if at least a single breakpoint exists in a notebook, right?
If this is not the case and you always run in debug mode, I wonder how big the performance penalty on cell execution would be...

Another thing to consider: running into an exception might result in a different (UX) experience in debug mode than in non-debug mode.

@roblourens
Copy link
Member Author

In option 3, yes I would only run in debug mode when there is a breakpoint in the cell or document because there is a perf hit to running with the debugger enabled.

Another thing to consider: running into an exception might result in a different (UX) experience in debug mode than in non-debug mode.

I forgot to call that out, thanks. Yes the "break on exception" behavior is weird for option 3 because we wouldn't really have a way to enable that when you don't have a breakpoint set...

Wouldn't 3 require checking whether there are any breakpoints, period?

That's also a good point. If I set a breakpoint in a .py file, and run a cell that calls into that file, I don't know how I will know to enable the debugger.

When do we render the editor glyph margin so users can add breakpoints? All the time or only while debugging or?

We just changed to showing it all the time (we removed the folding icon margin to do that)

I think pressing F5 should run by line

It's a good shortcut, I'd be worried about stealing it from the debug viewlet though. Then I can't start a debug session normally when a notebook is open.

https://github.com/microsoft/vscode-docs/blob/vnext/release-notes/images/1_59/run-by-line.gif

@greazer
Copy link
Member

greazer commented Aug 4, 2021

For 3, I think we have to keep in mind that even if debugging Python doesn't incur a perf hit (which based on my limited understanding, it does), other languages we want to support may very well have this problem (update: missed @pfitzseb comment that Julia does incur a perf hit for sure). That makes this option as a generalized model not so great.

Second, like @weinand and @pfitzseb, I'm also wondering about confusion as to what to expect if there are breakpoints elsewhere in the notebook.

#%%
    def foo(a):
🔴  print(a*2)

#%%
a = 10
foo(a)

A debugger savvy user would likely expect if you debug the 2nd cell, that the breakpoint would hit when you step over the call to foo(a). So I think we'd always have to run in debug mode. However, if the language being debugged does incur a perf hit this isn't going to be all that great. Of course we could just say that you have to add a breakpoint in the current cell in order to hit a breakpoint in another cell, or as @rchiodo points out, you could just always stop at the first line. Both of these options seem like they could be quite annoying for seasoned debugger users.

Finally, if we were to keep the gutter open for breakpoints all the time, I'm 100% sure that we will have people accidentally setting them and not having any clue as to what they just did and not being happy about it. Depending on the behavior after that, even more confusion could occur.

Therefore, while I like the idea, I don't really see how we can pull it off in a way that won't be problematic for many.

So while option 2 doesn't feel like nirvana it does offer a way to deal with the above disadvantages in a cleaner way.

  • If there is little or no perf hit when debugging, we could make this a sticky setting. I.e. users could turn it on and then it's always on, thus you get option 3, but in a conscious way.
  • If there is a perf hit then, we wouldn't want to have it on by default, and run-by-line is the only way to debug. This will likely work for the larger majority of users who even think about debugging notebooks, proper. We could also still make the option sticky for users who don't care about the perf hit. They'd just need to have an obvious signal that debugging is on so that they won't forget about it.
  • With the overall mode switch we could consider either hiding the run-by-line button altogether when debugging is enabled, or use that as a signal to replace the run-by-line button with a debug-cell button. Whether they appear in the consolidated run-button is up to the user (this is a side opinion about not hiding run-by-line in the consolidated run button by default. If users don't see it, they won't use it, and it breaks the spirit of run-by-line. Yes, then we have to think about the cell toolbar location problem, though).

@roblourens
Copy link
Member Author

Yes, there is a perf hit for Python

It would make sense to say that we run in debug mode when any cell in the notebook has a breakpoint, that's also an option.

Finally, if we were to keep the gutter open for breakpoints all the time, I'm 100% sure that we will have people accidentally setting them and not having any clue as to what they just did and not being happy about it

I haven't heard of this being a problem in editors in general. If we want to want to only show breakpoints in debug mode, then 2 is probably the only option that can do that.

@DavidKutu
Copy link

I like option 3, but there's a lot of scenarios that people have brought up that doesn't make it ideal. But I'd like to put it in as an option, with a setting people that don't care about performance can use it all the time.

As to when does option 3 activate, there's no right answer here. Its either compromise the full debugger or take the perf hit. I say we have start the debug session when we start the kernel, and we hit all breakpoints, even in other files. But we do it with a setting that is off by default, and if its off, we rely on option 2.

@davidanthoff
Copy link
Contributor

I think I'm also not too keen on 3. The performance issue @pfitzseb mentioned is the big one for Julia, but I also think there are some UI challenges that would be hard to overcome or make that not an ideal experience:

  • Say I have set 15 breakpoints, in my notebook and in other files that are run from within the notebook. So at this point, whenever I run a cell, I would be in debug mode. Realistically we would now need a toggle to disable all breakpoints at once, so that I can quickly switch to non-debug mode. I feel this is essentially introducing a "debug mode" button like in proposal 2, it just would be labeled in a less discoverable way and the logic of understand when debug mode would be on and off is just more complicated. There are also some other UI difficulties: say I have individually disabled 5 of my breakpoints. Now I hit the button "disable all breakpoints", and then after a while I hit "enable breakpoints" again. What happens now? Are all 15 breakpoints enabled? Only the 10 that were enabled before I disabled all of them?
  • I think the "check breakpoints to decide about debug mode" doesn't really work well with the concept of having multiple notebooks open at the same time, each with its own kernel. I very often would want to debug one notebook, but run another one just normally at the same time. For that I need a per notebook/kernel way of debugging or not debugging. But the presence of breakpoints is not per notebook, it is workspace global, so I think that would just be too coarse.
  • I'm not keen to make the presence of breakpoints in the current cell the deciding factor for enabling the debugger or not. Seems entirely legit that I want to run code under the debugger and only break in some code that is somewhere entirely different (i.e. different file).

I initially thought 1 might be nice, but I think in the end I agree that the worry of too many buttons, too many permutations of different run modes with debug mode etc. also doesn't make that a very attractive option.

So, here is a vote for option 2 :)

There is another reason that I think makes 2 a good option: in the Julia extension we also have as another major way to run code the Julia REPL. It is exposed as a terminal in VS Code, and users can run code there either by just typing it in, or sending code from a text editor to the REPL . At the moment we expose debugging in the REPL via macros: if you prefix your code with either @run or @enter, then we will attach the debugger and then run that individual piece of code in the debugger. That works, but if we moved to option 2 for notebooks, we could also move to a similar design for the REPL: we could expose UI to put the entire REPL into debug mode, and then any code you run there runs under the debugger, until you turn the debugger off for the REPL again, exactly like it works for a notebook. I think that kind of symmetry would be very nice.

@rchiodo
Copy link
Contributor

rchiodo commented Aug 4, 2021

Sorry I'm changing my vote. Thinking about the user scenario (not really worrying about impl details), debugging a cell is just like debugging a python file (or a c# project, or a exe). I would expect to hit a new button. Which I believe is option 1.

So there is a single command - 'Debug Cell' (like we have for the interactive window) that just starts the debugger.

Option 3 sounds cool, but is confusing from a user point of view. When am I debugging? When am I not?

Option 2 is weird because it introduces some sort of mode that nothing else has. Essentially it feels like we're exposing an implementation detail (that we have to attach to a kernel).

Option 1 sounds the most like other scenarios. I'm changing my vote to # 1.

@greazer
Copy link
Member

greazer commented Aug 4, 2021

Not sure how popular debugging has become in the Jupyter Lab world, but because it is a large ecosystem, there are now potentially a lot of notebook users (even those who are fully comfortable with debugging) who will look for and feel at home with a separate mode. We should at least keep that in mind.

Oh, was gonna mention too that I'll bet that Juptyer Lab went through many of the very same conversations as they were defining what to do for debugging (maybe there's another public discussion about it?)

Overall, I think the main decision so far is focusing around at least not having debugging on all the time without the user having any obvious control.

@rchiodo
Copy link
Contributor

rchiodo commented Aug 4, 2021

Oh, was gonna mention too that I'll bet that Juptyer Lab went through many of the very same conversations as they were defining what to do for debugging (maybe there's another public discussion about it?)

My bet is the opposite. The implementation details crept into the UI. They knew they had to attach all the time. The easiest way to handle that is just have a button that essentially attaches.

@greazer
Copy link
Member

greazer commented Aug 4, 2021

Oh, and to @roblourens questioning about users accidentally hitting the gutter and setting breakpoints in regular editors. First, I really don't know that this is a problem worth worrying about. It's just a guess on my part. :)

But the reason I'm guessing this is because notebooks are much more "clickable" than regular editors. There's widgets all over the place. In addition, the click area to make a cell editable doesn't really correspond with anything in a regular editor since you're always in edit mode. Finally, the area tends to be much smaller in general, so there's just simply more chance that this could occur.

@greazer
Copy link
Member

greazer commented Aug 4, 2021

Is toggling the same as attach? While it may be possible for Python, it isn't necessarily true for other runtimes/languages. Some kernels will likely require starting them under a debug mode. So having a debug button that's readily available on a cell may not really be viable for all languages. I.e. it would be weird, if not a non-starter, if after clicking it, the user is prompted with "Oh, you have to restart your kernel to debug this cell". It's also weird if it gets grayed out or disappears.

A global switch for this type of kernel speaks to the likely need to be starting multiple sessions while debugging your notebooks. I.e. I don't necessarily want to always have to remember to run the first cell under the debugger in case I want to debug a cell later. I suppose the same could be said for run-by-line though... It's just that run-by-line doesn't carry the same weight or expectations that debugging does.

@rchiodo
Copy link
Contributor

rchiodo commented Aug 4, 2021

So you're suggesting that it's possible that option 2 will cause the entire kernel to restart? Instead of having the kernel support attach?

I don't think that case is necessary/likely. And optimizing for that UI flow is a bad choice. We should just say all kernels require attach support. The option 2 button is already implemented that way today.

                .startDebugging(
                    undefined,
                    {
                        type: DataScience.pythonKernelDebugAdapter(),
                        name: name,
                        request: 'attach',
                        internalConsoleOptions: 'neverOpen',
                        __document: document.uri.toString()
                    },
                    options
                )

@rebornix
Copy link
Member

rebornix commented Aug 4, 2021

My two cents: introducing new buttons and keyboard shortcuts for debugging is not popular but that's not what we can avoid. When there are breakpoints in the cell editor, we still need a way to just run the cell without entering debugging mode (hitting breakpoints), either through a separate button, or a new keyboard shortcut. Thus I vote for option 1. Either option 2 and option 3 will degrade the experience of this scenario.

My personal preference is even beyond this: ctrl+enter should always just run, no matter if there is a breakpoint or not, and if I want to debug, I would use mouse or have another keybinding. Sacrificing execution performance for debugging is also not appealing.

@davidanthoff
Copy link
Contributor

At least for Julia the attach option is not a problem, i.e. there wouldn't be a need to restart the kernel to start debugging.

Could option 1 and 2 both be implemented? They don't necessarily strike me as mutually exclusive. There could be a button "debug this cell" that attaches the debugger and runs that cell and then automatically detaches the debugger from the kernel, and there could also be an option to attach the debugger for longer to a kernel, i.e. option 2.

I kind of think that option 1 is great for the simplest scenario where I want to debug one cell, but what about "run all cells above", "run all cells below", "run cell and insert a new cell below", "run cell and don't insert a new cell below" etc. Do they all get a debug version as well? Obviously they can't really, but I can easily see a scenario where I would want to use those with debugging, and then option 2 starts to look nice.

In the Julia extension we have this view of all the running kernels:

image

We could pretty easily add a button at the location where the red arrow is that attaches a debugger to that given kernel. I think that in combination with say a "Debug this cell" button might be a pretty nice UI. You have an easy option right in your face, and a slightly more advanced version also available.

@davidanthoff
Copy link
Contributor

One more random observation: I think what really tripped me with the "attach debugger to this notebook" examples that I've seen previously is that they used an icon in a place where normally the "run this file" button appears, and the icon also looked a bit like "run something". I think the problem there is if the command to attach the debugger to the notebook looks or feels similar to something that "runs" something, because it of course doesn't. So maybe just making sure the icon looks like "attach" and not like "run", and is not in the same location where we normally find "run" buttons would help.

@greazer
Copy link
Member

greazer commented Aug 4, 2021

We should just say all kernels require attach support.

This seems reasonable, provided we have consciously made that decision and statement.

@roblourens
Copy link
Member Author

We should just say all kernels require attach support

The way we are doing this right now, we are implementing everything in the vscode-jupyter extension, contributing the UI from the extension, and are only concerned about the python kernels that this extension returns. I'm not really keen on making vscode opinionated about how notebook debugging works right now. Maybe later we could save extensions some effort by helping them hook into a common UI, when we have a more clear vision.

I think the clear path forward for now is just implementing option 1 as an experiment to see how it feels. I thi.nk that option 3 is not going to happen based on the discussion, although I just talked about it with @DonJayamanne last night and it sounds like he might implement it that way for his node notebook, which is great, more experimentation and more data.

Maybe to start out I would only have one new "Debug Cell" button which is equivalent to the normal "Run cell". It can go under the run button chevron all the time, and the setting that exists for that can just control whether the "Run above/below" buttons go in there too. And I'll try to come up with a keyboard shortcut for it.

@davidanthoff
Copy link
Contributor

That sounds great.

I think I'll go ahead and try to implement the "attach debugger to kernel" with the UI I outlined above for the Julia controller, and also do the "Debug cell" thing at the same time.

One question for @roblourens: I assume you will add the "Debug Cell" button as part of the Jupyter extension? Would that button than somehow not be visible if a user selects the Julia notebook controller from our extension? And then we could also provide our own "Debug Cell" button? Or would we somehow "share" one "Debug Cell" button?

@roblourens
Copy link
Member Author

I assume you will add the "Debug Cell" button as part of the Jupyter extension? Would that button than somehow not be visible if a user selects the Julia notebook controller from our extension? And then we could also provide our own "Debug Cell" button? Or would we somehow "share" one "Debug Cell" button?

At least for now, yes, the jupyter extension would contribute the button and it would only appear for kernels that we contribute and have decided we can debug. In the future if we wanted to standardize that UI, maybe we would make something shared that your extensions or others could hook into.

@roblourens
Copy link
Member Author

Calling this resolved, and any other work will be done via bugs/feature requests.

@greazer greazer modified the milestones: August 2021, September 2021 Aug 31, 2021
@github-actions github-actions bot locked and limited conversation to collaborators Oct 9, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
feature-request Request for new features or functionality notebook on-testplan
Projects
None yet
Development

No branches or pull requests

11 participants