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

Disallow file and folder names that end with dots #7772

Merged
merged 3 commits into from
May 20, 2014
Merged

Conversation

JeffryBooher
Copy link
Contributor

Fixes:
#5468 (although I was never able to entire reproduce this case)
#7224

This will allow for the user to create dots at the end of a file or directory name using the OS (finder/explorer) and Brackets will honor those but, because of the complexities of how the OS treats trailing dots in entity names, we disallow the user from creating entities that end with dots.

This is not for Sprint 39

@JeffryBooher JeffryBooher changed the title Disallow file and folder names that end with dots [REVIEW ONLY] Disallow file and folder names that end with dots May 7, 2014
@JeffryBooher
Copy link
Contributor Author

Be careful when testing this. Folders with trailing dots are impossible to get rid of. The operating system will brackets and windows explorer create them but not delete them. You will get "The specified path does not exist" so create them in a folder that you can later delete.

I mistakenly created them in the brackets/src folder and it took me a little bit of trial and error to get rid of them.

@JeffryBooher JeffryBooher changed the title [REVIEW ONLY] Disallow file and folder names that end with dots Disallow file and folder names that end with dots May 13, 2014
@peterflynn
Copy link
Member

@JeffryBooher Just figured out why I was confused in the standup :-) Files and folders don't behave the same way:

  • If you try to create a file ending in ".." in Explorer or Brackets, Windows strips the dots off -- it seems 100% impossible to create a file with such an ending. The only issue is that Brackets sees the call as succeeding, so it adds a non-existent ".."-suffixed entry to the project tree (which appears next to the real, stripped filename -- that one gets added when file watchers notice the creation on disk).
  • If you try to create a folder ending in ".." in Explorer, it behaves as above (stripes off the ".."). If you do this in Brackets however, it actually creates the file. Even though some parts of Windows will choke on the folder name.

So it seems like the fix should be the same in both cases, though: disallow creation in the first place, as this PR does.

@peterflynn
Copy link
Member

Btw @JeffryBooher, how do you get rid of a ".."-suffixed folder once you've created one? :-)

@JeffryBooher
Copy link
Contributor Author

@peterflynn yeah this was a PITA for me. So Make sure it's in a folder that you can delete and then you have to remove the parent folder and ignore errors. I think it's del -f -s -q <parent-folder> on the command line

StringUtils.format(Strings.INVALID_FILENAME_TITLE, isFolder ? Strings.DIRECTORY : Strings.FILE),
StringUtils.format(Strings.INVALID_FILENAME_MESSAGE, _invalidChars)
StringUtils.format(Strings.INVALID_FILENAME_TITLE, isFolder ? Strings.DIRECTORY_NAME : Strings.FILENAME),
StringUtils.format(Strings.INVALID_FILENAME_MESSAGE, isFolder ? Strings.DIRECTORY_NAMES_LEDE : Strings.FILENAMES_LEDE, _invalidChars)
Copy link
Member

Choose a reason for hiding this comment

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

It seems like a bunch of these string diffs are just here to change the dialog message from "file name" to "filename." I think "file name" is actually better -- and it seems like it would simplify the diff too.

CC @njx for the string question

Copy link

Choose a reason for hiding this comment

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

I'm very used to "filename" (and that's how we tend to treat it in code - we don't inner-cap the N). For user-facing strings "file name" is probably more standard English, though for technical users it might be different - I have a suspicion that coders are probably more used to "filename", though I don't have any data.

(FWIW, I did a search in Adobe's help docs, and it seems like the documentation itself usually uses "filename", but when it refers to label strings in the UI they tend to be "File Name", probably because they need to be capitalized and "Filename" looks weird. That doesn't apply to this case anyway since it's uncapitalized in a message string, not being used as a label.)

Anyway, I guess after all that I don't have a strong opinion either way :), as long as we're consistent. I didn't see any other user-visible uses of "filename" in the current strings file, so I think this one could go either way, and we can follow it elsewhere. Personally, I think it's fine to keep it as "filename".

Copy link
Member

Choose a reason for hiding this comment

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

@njx Actually it looks like all of these should be uppercase, since they're being used in a dialog title. I think it's a bug in the PR (and on master!) that they're lowercase.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@peterflynn Well, I changed this because you're right we should be Title Case on the titles but having trouble testing it. I tried removing all permissions on a folder except for "Read" and renaming a file I ran into "Error renaming file" then I noticed we also have "Error deleting file", "Error loading project". I'm not sure how deep this issue goes and it could go kind of deep.

We could try to fix all of them, just this occurrence or raise an issue to fix them all at once. We could attempt to fix them using a css text-transform (text-transform:capitalize) but I have no idea how that works with localization. It may not work or not work it incorrectly in some scenarios.

@JeffryBooher
Copy link
Contributor Author

@redmunds go ahead and give this a final review and merge if it's ready. I'll move the discussion of UI to another issue so we can capture the larger 1.0 UI Audit

@redmunds
Copy link
Contributor

LGTM. Merging.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants