Add FilePathUtils module

[core-ai-bot]

Issue by redmunds
Thursday Apr 24, 2014 at 21:19 GMT
Originally opened as adobe/brackets#7631

---

This is code cleanup of a circular dependency introduced here and discussed here.

Proposed solution:

1. Create a new FilePathUtils module for file path utilities and keep utilities for actual file handling in FileUtils. API for PathUtils:

   convertToNativePath
   convertWindowsPathToUnixPath
   getNativeBracketsDirectoryPath
   getNativeModuleDirectoryPath
   canonicalizeFolderPath
   stripTrailingSlash
   getDirectoryPath
   

2. Review FileUtils and LanguageManager APIs to verify functions are where they belong

3. Remove FileUtils dependency from LanguageManager (and replace with FilePathUtils).

4. Move to LanguageManager:

   isStaticHtmlFileExt
   isServerHtmlFileExt
   

5. Remove _staticHtmlFileExts and _serverHtmlFileExts from FileUtils. Use LanguageManager instead.

6. Deprecate changed APIs.

[core-ai-bot]

Comment by peterflynn
Monday Apr 28, 2014 at 18:18 GMT

---

Reviewed - low priority@redmunds, not needed for 1.0

[core-ai-bot]

Comment by chirayu11
Wednesday May 28, 2014 at 06:09 GMT

---

@redmunds file/FileUtils.js does not have a dependency on language/LanguageManager.js. I am on the latest commit on Master branch. Is the circular dependency still there?

[core-ai-bot]

Comment by redmunds
Wednesday May 28, 2014 at 15:17 GMT

---

@chirayu11 Yes, the circular dependency is still there. You can see the require() call here: https://github.com/adobe/brackets/blob/master/src/file/FileUtils.js#L37

[core-ai-bot]

Comment by le717
Saturday Feb 14, 2015 at 21:00 GMT

---

@redmunds Was just trying to work on this, but some things have changed since you filed this:

• LanguageManager only relies on FileUtils.getBaseName(), which is not on your list of methods in the new FilePathUtils module, so the circular dependency is not resolved by making this.
• isStaticHtmlFileExt() and isServerHtmlFileExt() both require FileUtils.getFileExtension(), which again is not part of FilePathUtils and does not resolve the issue.
• FileUtils.comparePaths(), which would be moved to FilePathUtils, relies on FileUtils.compareFilenames(), but it seems as it may be fine as that is the only thing the whole new module relies on, and nothing in FileUtils uses FilePathUtils.

In light of this, what do you suggest I do?

[core-ai-bot]

Comment by le717
Saturday Feb 14, 2015 at 21:03 GMT

---

Excuse me, the last point would not be fine as deprecating the functions in FileUtils would introduce a dependency on FilePathUtils, which is obviously circular and partly defeats the purpose of making FilePathUtils until the warnings are removed.

[core-ai-bot]

Comment by redmunds
Saturday Feb 14, 2015 at 21:12 GMT

---

@le717 The goal for FilePathUtils is to move all functions that only operate on the string values of file paths, names, extensions, etc. and do not do any file i/o operations. It's been a while since my original proposal, so that list may no longer be accurate.

• LanguageManager only relies on FileUtils.getBaseName(), which is not on your list of methods in the new FilePathUtils module, so the circular dependency is not revolved by making this.
• isStaticHtmlFileExt() and isServerHtmlFileExt() both require FileUtils.getFileExtension(), which again is not part of FilePathUtils and does not resolve the issue.
• FileUtils.comparePaths(), which would be moved to FilePathUtils, relies on FileUtils.compareFilenames(), but it seems as it may be fine as that is the only thing the whole new module relies on, and nothing in FileUtils uses FilePathUtils.

Based on that criteria, it looks like FileUtils.getBaseName(), FileUtils.getFileExtension(), and FileUtils.compareFilenames() should also be moved to FilePathUtils.

[core-ai-bot]

Comment by le717
Saturday Feb 14, 2015 at 22:37 GMT

---

Based on that criteria, it looks like FileUtils.getBaseName(), FileUtils.getFileExtension(), and FileUtils.compareFilenames() should also be moved to FilePathUtils.

Moved.

The goal for FilePathUtils is to move all functions that only operate on the string values of file paths, names, extensions, etc. and do not do any file i/o operations.

In that case, it seems most of the contents in this module will be moved to FilePathUtils. However, doing everything may take one primary PR and a few smaller ones to finish. Also, I would assume this intention would include moving FileUtils.getSmartFileExtension(), as it works on the file extension string, but it too relies on LanguageManager (LanguageManager.getLanguageForExtension()), and that is a core part of LanguageManager.

It almost sounds as if LanguageManager needs some breaking up somehow. 😕

[core-ai-bot]

Comment by redmunds
Monday Feb 16, 2015 at 16:30 GMT

---

I would assume this intention would include moving FileUtils.getSmartFileExtension(), as it works on the file extension string, but it too relies on LanguageManager (LanguageManager.getLanguageForExtension()), and that is a core part of LanguageManager.

Agreed, but based on issue #8042 (which I do not agree with), it sounds like we might get rid of FileUtils.getSmartFileExtension().

It almost sounds as if LanguageManager needs some breaking up somehow.

LanguageManager still seems coherent to me.