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

Support mkdirp when saving files #48062

Closed
jrieken opened this issue Apr 17, 2018 · 7 comments
Closed

Support mkdirp when saving files #48062

jrieken opened this issue Apr 17, 2018 · 7 comments
Assignees
Labels
debt Code quality issues file-io File I/O

Comments

@jrieken
Copy link
Member

jrieken commented Apr 17, 2018

There are scenarios, like creating .vscode/launch.json that assume we always run mkdirp-logic first. While that make sense for new files it doesn't make sense for files we have opened before, e.g. files users edit. Too address these scenarios I have added IUpdateContentOptions#mkdirp and the RemoteFileService honours that flag, the default file service always runs mkdirp I believe (cc @bpasero).

@jrieken
Copy link
Member Author

jrieken commented Apr 17, 2018

/cc @weinand This is for the debug scenario.

@jrieken jrieken added debt Code quality issues file-io File I/O labels Apr 17, 2018
jrieken added a commit that referenced this issue Apr 17, 2018
@weinand
Copy link
Contributor

weinand commented Apr 17, 2018

/cc @isidorn

@bpasero
Copy link
Member

bpasero commented Apr 20, 2018

@jrieken @weinand what is the scenario in debug where this is relevant. e.g. if we keep doing mkdirp for any file:// operation as before, what is the issue from that behaviour?

And who would set mkdirp to false currently or where would we want to add it?

@weinand
Copy link
Contributor

weinand commented Apr 20, 2018

@bpasero for debug there is no issue. The problem is that debug was relying on "mkdirp" when creating ".vscode/launch.json". But the remote filesystem provider doesn't use "mkdirp", so our code was failing there.
So if we have to implement a fallback strategy anyway, there is no point in supporting "mkdirp" for some filesystems but not for all.

@jrieken
Copy link
Member Author

jrieken commented Apr 20, 2018

set mkdirp to false currently or where would we want to add it?

You wanna set it to true, e.g when creating files but IMO it is not needed when saving a files that you have just opened.

@bpasero
Copy link
Member

bpasero commented Apr 23, 2018

@jrieken I think that depends. the file you have opened could have been deleted (including its parent folders) and our current behaviour is to restore the full path and file when you save it. Otherwise the behaviour would be weird:

  • make a file dirty
  • delete it on disk and its parent folders
  • save it

=> you would get an error if mkdirp was set to false

@jrieken
Copy link
Member Author

jrieken commented Apr 27, 2018

We now do that...

@jrieken jrieken closed this as completed Apr 27, 2018
@vscodebot vscodebot bot locked and limited conversation to collaborators Jun 11, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
debt Code quality issues file-io File I/O
Projects
None yet
Development

No branches or pull requests

3 participants