-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Fixes #4741 allow creation of sub-files and/or sub-folders if name has "/" #4764
Conversation
const childUri = new URI(parent.uri).resolve(name).toString(); | ||
this.fileSystem.exists(childUri).then(exists => { | ||
if (exists && dialog) { | ||
dialog.error(`A file or folder <strong>${fileName}</strong> already exists at this location.`); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should change a type of validation function to MaybPromise<DialogError>
to allow async validation. Inside the dialog we will need to make sure that concurrent validation does not occur, i.e. queue them or cancel.
I wonder can we override |
It does not work if i create the same path twice from the root, it seems to pick the wrong parent then which is very confusing. Not sure whether it is a new bug or something which was not apparent before. In order to reprdoce with Theia repo:
I would expect an error, but there are non and the second file is created at the bogus location: |
This is because of inconsistent selection bug when creating a new file. Sometimes the new file gets selected on creation (which is what it is supposed to do) and sometimes it doesn't. I've patched this on the latest commit. |
@@ -116,7 +116,7 @@ export class FileResource implements Resource { | |||
} | |||
|
|||
protected async getFileStat(): Promise<FileStat | undefined> { | |||
if (!this.fileSystem.exists(this.uriString)) { | |||
if (!await this.fileSystem.exists(this.uriString)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
how it used to work! ❤️ for fixing it!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It works nicely now, but there are some code clean up necessary to avoid breaking code org and race conditions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
works nicely, thank you
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks @uniibu I didn't expect someone will implement a fix for this issue very soon
This allow creation of files and folders recursive path. improve validation check, add cancellationtoken separate validation for absolutpath and leading/trailing spaces Signed-off-by: Uni Sayo <unibtc@gmail.com>
i've separated the validation checks for absolute paths and leading and trailing spaces as per suggested. Squashed commits. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks 👍 @uniibu
Fixes #4741. Creation of sub-files and/or sub-folders are already available,however, validation check does not allow
/
on the input box, making it impossible to create nested sub-folders and/or sub-files.This PR allows recursive creation of folders or files via
/
./
Example:
test/scripts/index.js
will create test andtest/scripts
folder if it still does not exists and lastly it creates the filetest/scripts/index.js
Signed-off-by: Uni Sayo unibtc@gmail.com