Run line by line in an untitled file prompts to save file

[miguelsolorio]

Testing microsoft/vscode#129467

CleanShot.2021-07-27.at.14.41.02.mp4

I'm not sure what is expected here, but I got prompted to save an untitled notebook (with no workspace) but also clicked "cancel" and then it kept debugging? Either it should only work if you save it or allow to debug (I vote for the latter).

[roblourens]

@isidorn can we suppress the "save all" when a debug session starts? Maybe another startDebugging option?

[isidorn]

@roblourens you should do the same as powershell. More details and an example how to fix here PowerShell/vscode-powershell#3338
So you should just do that for .ipynb files

[roblourens]

Hey @isidorn, that solution doesn't work because the language here is still Python, and we can't do it based on file extension. We don't want this for all Python debugging, just specifically in a notebook, so I think we need a flag on the launch config to set this dynamically. Does that make sense? It would be similar to the "simple UI" flag

[roblourens]

cc @weinand

[isidorn]

@roblourens a flag in the debugSessionOptions would work. However I am thinking if we can do this without introducing any new concepts.
Or we could piggy back on the simpleUI flag and say if it is a simpleUI that means we are not saving untitled files before...
Code pointer https://github.com/microsoft/vscode/blob/main/src/vs/workbench/contrib/debug/browser/debugService.ts#L315

[weinand]

I don't like piggybacking on the simpleUI flag because it waters down the semantics of the flag (not saving files is not really a feature of a "simple UI").

I would rather introduce a new mechanism for this.

A new flag in the debugSessionOptions is a pragmatic approach (which we should use here).

What I don't like about this approach is that the caller of the startDebugging API does not know, whether the underlying debugger/runtime supports debugging of unsaved files or not. So the caller is not really free to make an arbitrary decision. In the notebook case at hand this is not a problem because here the caller knows for sure that the save is not needed. But in general this is not the case...

It would be helpful if we could tie the "debug.saveBeforeStart" setting to "more than a language", basically a "when" context condition.

Similar for what is done for PowerShell:

"configurationDefaults": {
      "[powershell]": {
        "debug.saveBeforeStart": "nonUntitledEditorsInActiveGroup"
      }
},

we could do something like this for Python in notebooks:

"configurationDefaults": {
      "[alnguage == 'python' && editor == 'notebook']": {
        "debug.saveBeforeStart": "nonUntitledEditorsInActiveGroup"
      }
},

But this is just dreaming...

[isidorn]

fyi @sandy081 for a more powerful settings scoping mechanism. Also in case there might be some other mechanism to use here.

[roblourens]

Yeah, we previously had a conversation about making the setting scopes more powerful so we can set settings based on notebook kind.

I was thinking, is it ever useful to save an untitled file when I start debugging? Even if it's a .py file, it's not going to be imported by other files or in a launch.json if it is an untitled file that I just created. Can we save editors for files with unsaved changes but not save untitled files?

Alternatively, I'm not sure how to express what I want as a more complex option for saveBeforeStart. Maybe "activeEditorIsNotNotebook". Otherwise a debugSessionOptions would be best. And yeah the simpleUI flag also can't be reused because we want the same for full debugging.

[isidorn]

@roblourens yeah we explicitly added saving untitled files for beginners. Here's the scenario: a new user open a new untitled file, types some script and presses F5 to debug it The untitled file is not saved, and a lot of participants did not know that they have to save a file in order to debug it. So with the current approach we nicely prompt the user to save and then we start debugging. I would not change this flow.

Making saveBeforeStart more powerful with activeEditorIsNotNotebook sounds good to me. Though I would prefer not to have a notebook in the setting value. So something like activeEditorIsRegularEditor or whatever the name is for regular editors.

[roblourens]

I guess a problem is that in a language scope we would be setting this for all python files, and if the user configured this as something else that would override our setting. Given that, if we don't have a config scope for notebooks then it seems impossible to get it right with a setting.

[isidorn]

@roblourens you are correct, however that is a corner case. We can check numbers, but I think that a very low number of users actually touch this setting.

[weinand]

Yes, a setting tries to solve the problem on the wrong end: the user doesn't know the notebook cell debugger's requirements. Only the notebook cell debugger knows (this is already a problem for the PowerShell solution which ties the save requirement to the language instead of the debugger).

Since we don't have a contribution mechanism to communicate that information to the "save" code, using a debugSessionOptions seems to be the closest solution for now.

[roblourens]

the user doesn't know the notebook cell debugger's requirements. Only the notebook cell debugger knows

Well said, I agree that it should go in debugSessionOptions, I can implement that.

[weinand]

The only remaining question is how the debugSession option and the setting interact.
IMO the setting takes precedence over the debugSession option. So a user could overrule the native behavior. But I'm not so sure about this...

[isidorn]

I think the configurationService does not let us know if the setting has been set by the user or it is just providing the default.
So for pragmatic reasons I suggest that the debugSessionOption takes priority.

[weinand]

@isidorn yeah, that makes sense

[sandy081]

Jumping in here (not having previous context)

I think the configurationService does not let us know if the setting has been set by the user or it is just providing the default.

You can use inspect api of configuration service to know if a setting is defined by user or not.

[roblourens]

Added a flag called suppressSaveBeforeStart?: boolean; to DebugSessionOptions, let me know if that sounds ok

[isidorn]

Sounds good to me.

[DonJayamanne]

The run by line icons is missing in untitled files.

• Open a workspace
• Create a new untitled notebook using the new Create: New file command
• You can see that the kernel is picked (I think this is because i had used a kernel for untitled files & vscode remembers this)
• However run by line icon is missing.

Note: Opening an existing file will fix this (however if i wanted to try just RBL with a blank notebook, then its broken).

See below (kernel picker has the kernel & its a python cell)

[roblourens]

I see this context key: jupyter.ispythonnotebook: false