Skip to content
This repository was archived by the owner on Sep 6, 2021. It is now read-only.

Directory creation error #4244

Merged
jasonsanjose merged 10 commits into from
Jul 11, 2013
Merged

Directory creation error #4244

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
53ed7ee
Fixed directory already exists issue
thefirstofthe300 Jun 14, 2013
33e01f3
Merge branch 'duplicate-directory-dialog-fix' into directory-creation…
thefirstofthe300 Jun 15, 2013
6731218
Implemented code review suggestions
thefirstofthe300 Jun 28, 2013
29cd68d
Finished code review fixes
thefirstofthe300 Jun 28, 2013
e17c0a7
Added variables to strings
thefirstofthe300 Jun 28, 2013
de84314
Merge branch 'master' into directory-creation-error
thefirstofthe300 Jul 9, 2013
988e637
Removed Directory_Already_Exists and adopted comments
thefirstofthe300 Jul 9, 2013
0f26eba
Implemented more comments
thefirstofthe300 Jul 10, 2013
b87a928
Fixed typo that broke error_cleanup
thefirstofthe300 Jul 11, 2013
ca269fa
Implemented final comments (I hope)
thefirstofthe300 Jul 11, 2013
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/nls/root/strings.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,10 @@ define({
"ERROR_DELETING_FILE_TITLE" : "Error deleting file",
"ERROR_DELETING_FILE" : "An error occurred when trying to delete the file <span class='dialog-filename'>{0}</span>. {1}",
"INVALID_FILENAME_TITLE" : "Invalid file name",
Copy link
Member

@jasonsanjose jasonsanjose Jun 17, 2013

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't we just change the 2 strings INVALID_FILENAME_TITLE and FILE_AREADY_EXISTS to take a parameter? e.g.

StringUtils.format(Strings.INVALID_FILENAME_TITLE, Strings.FILE)
...
StringUtils.format(Strings.FILE_ALREADY_EXISTS, Strings.DIRECTORY, StringUtils.breakableUrl(data.rslt.name))

We would probably want to tackle the ERROR_DELETING_FILE message as well in the same way.

"INVALID_DIRECTORYNAME_TITLE" : "Invalid directory name",
"INVALID_FILENAME_MESSAGE" : "Filenames cannot contain the following characters: /?*:;{}<>\\| or use any system reserved words.",
"FILE_ALREADY_EXISTS" : "The file <span class='dialog-filename'>{0}</span> already exists.",
"DIRECTORY_ALREADY_EXISTS" : "The directory <span class='dialog-filename'>{0}</span> already exists.",
Copy link
Member

@jasonsanjose jasonsanjose Jul 9, 2013

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You probably intended to remove DIRECTORY_ALREADY_EXISTS right?

"ERROR_CREATING_FILE_TITLE" : "Error creating file",
"ERROR_CREATING_FILE" : "An error occurred when trying to create the file <span class='dialog-filename'>{0}</span>. {1}",

Expand Down
27 changes: 19 additions & 8 deletions src/project/ProjectManager.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1255,14 +1255,25 @@ define(function (require, exports, module) {
var errorCallback = function (error) {
if ((error.name === NativeFileError.PATH_EXISTS_ERR) ||
(error.name === NativeFileError.TYPE_MISMATCH_ERR)) {
Copy link
Member

@jasonsanjose jasonsanjose Jul 11, 2013

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Almost. I went back to the original bug to test and found some more issues not mentioned.

If you create a file with a name foo, the try to create a directory of the same name, you get "The directory foo already exists" instead of "The file foo already exists"...and vice versa.

You can split the if here with an else if branch to handle TYPE_MISMATCH_ERR separate from PATH_EXISTS_ERROR, that should fix it.

Dialogs.showModalDialog(
DefaultDialogs.DIALOG_ID_ERROR,
Strings.INVALID_FILENAME_TITLE,
StringUtils.format(
Strings.FILE_ALREADY_EXISTS,
StringUtils.breakableUrl(data.rslt.name)
)
);
if (selectionEntry.isFile) {
Copy link
Member

@jasonsanjose jasonsanjose Jun 17, 2013

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove this if statement new if statement and just use suggestions above.

Dialogs.showModalDialog(
DefaultDialogs.DIALOG_ID_ERROR,
Strings.INVALID_FILENAME_TITLE,
StringUtils.format(
Strings.FILE_ALREADY_EXISTS,
StringUtils.breakableUrl(data.rslt.name)
)
);
} else {
Dialogs.showModalDialog(
DefaultDialogs.DIALOG_ID_ERROR,
Strings.INVALID_DIRECTORYNAME_TITLE,
StringUtils.format(
Strings.DIRECTORY_ALREADY_EXISTS,
StringUtils.breakableUrl(data.rslt.name)
)
);
}
} else {
Copy link
Contributor

@TomMalbran TomMalbran Jun 18, 2013

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At this else branch there is another error dialog that always display "File" in the strings as the code fixed. Maybe this could be fixed here too.

var errString = error.name === NativeFileError.NO_MODIFICATION_ALLOWED_ERR ?
Strings.NO_MODIFICATION_ALLOWED_ERR :
Expand Down