microsoft / vscode

Visual Studio Code
https://code.visualstudio.com
MIT License
164.19k stars 29.29k forks source link

New notebook from the command line issues #125028

Closed bpasero closed 3 years ago

bpasero commented 3 years ago

The workbench distinguishes untitled editors:

The former is the usual untitled editor that you get when you pick File > New File and the editor will not appear dirty until you start typing contents. The latter is for the scenario of the user typing code <some path>/<some file> pointing to a file that does not exist. We then:

This flow does not seem to work for notebooks yet. Steps to Reproduce:

  1. type code-insiders /<some path>/new.github-issues
  2. an untitled notebook opens

List of issues:

@jrieken for this purpose the IUntitledFileWorkingCopyManager provides an overload to the resolve method called INewUntitledFileWorkingCopyWithAssociatedResourceOptions. If you pass on the path as associated resource, everything should work fine.

image

//cc @lramos15

jrieken commented 3 years ago

@bpasero I have pushed a change to use associatedResource instead of untitledResource (in notebook/dev). I am slightly confused by that because the former seems to super-seed the latter and I am not sure why there is support for both in the first place...

Anyways, with this change new notebooks open just fine from the command line which is good. Tho, it seems the path isn't picked up properly when saving. E.g

bpasero commented 3 years ago

@jrieken yeah unfortunately we cannot just switch from _workingCopyManager.resolve({ untitledResource: this.resource }) to _workingCopyManager.resolve({ associatedResource: this.resource }) for all cases.

Here is the idea behind it, given the resolve API:

In that way associatedResource is like a flag telling VSCode how to treat the untitled file (e.g. not asking for a file path on save).

I don't have a good answer how to solve this today for notebooks actually, so maybe we want to move this out to June.

A somewhat hack solution would be to look at the incoming resource and understand whether it was generated by the notebook resolver service or not, but that would be just guessing.

A better way would be to know from the editor override what the original request to open has been, because there we know it and create the right untitled editor input either with associated resource or not.

For now I would go back to untitledResource as the other one is more a corner case I would say that is hit less often.

jrieken commented 3 years ago

For now I would go back to untitledResource as the other one is more a corner case I would say that is hit less often.

I don't think that needed. My understanding is the following

bpasero commented 3 years ago

@jrieken yeah you are right, I have some code to get a proper untitled resource.

However, we still wrongly assume the untitled file working copy has an associated resource even if it shouldn't due to this check:

https://github.com/microsoft/vscode/blob/da4fcc266fc68103b6e8b6e932f5df786cb6e82c/src/vs/workbench/services/workingCopy/common/untitledFileWorkingCopyManager.ts#L197-L197

The hasAssociatedFilePath is a public property on UntitledFileWorkingCopy that is used to drive a few things, like not asking for a file dialog when saving:

https://github.com/microsoft/vscode/blob/b8fe2db4392446a211767a00e8213d040786e54d/src/vs/workbench/services/workingCopy/common/fileWorkingCopyManager.ts#L377-L381

jrieken commented 3 years ago

Ok. I revert my change. Let's tackle this in June but tbh I wouldn't know when to use associatedResource and when to use untitledResource. This is soo deep down the rabbit hole that I need to break all encapsulation to bring in this information down

bpasero commented 3 years ago

Yeah no worries, I want to solve this in a clean way without hacks, especially https://github.com/microsoft/vscode/issues/124352 needs fixing too to ensure a nice experience, otherwise you end up with 1 hidden untitled text model lurking around even after save.

Adding Logan because this might depend on more override help.

bpasero commented 3 years ago

Verification: