[WIP] Introduce FilePathUtils module

[le717]

Part of #7631.

This introduces a new FilePathUtils module, which moves many APIs from the FileUtils in an attempt to resolve some of the circular dependencies.

• While not all functions have been relocated, this does break the circle between LanguageManager and FileUtils, with only the latter using the former.
• FileUtils.getSmartFileExtension() has yet to be moved, as it relies on LanguageManager.getLanguageForExtension(), which just moves the circle to a different module. This will need to be worked out before merging.
• Moving FileUtils.isStaticHtmlFileExt() and FileUtils.isServerHtmlFileExt() to LanguageManager will create a circular dependency between it and FilePathUtils during deprecation unless it is flat-out moved without redirect.
• I have not yet split/updated many of the affected unit tests.

I'm mainly posting this now to get feedback, review what stays in FileUtils and what is moved to FilePathUtils, and resolve any remaining issues.

/cc @redmunds

[redmunds]

@le717

• Moving FileUtils.isStaticHtmlFileExt() and FileUtils.isServerHtmlFileExt() to LanguageManager will create a circular dependency between it and FilePathUtils during deprecation unless it is flat-out moved without redirect.

I suggested moving them to LanguageManager to get rid of the _staticHtmlFileExts and _serverHtmlFileExts hard-coded lists of file extensions. But, to make that work a new client vs server document field will need to be added to language data. I think a temporary, well-documented circular dependency to handle deprecation is OK, but it might be worth separating that change out to another issue.

[le717]

I've moved _is*FileExt() to LanguageManager, but left getSmartFileExtension() alone as moving it makes the circular dependencies a bit more convoluted:

• LanguageManager => FilePathUtils
  • This poses no issues as is desired action
• FilePathUtils => LanguageManager
  • Not temporary, getSmartFileExtension() requires LanguageManager (though if it is removed, this problem goes away)
• FileUtils => FilePathUtils and LanguageManager
  • Both temporary and more permanent. FileUtils would only rely on FilePathUtils for deprecation, but would still require LanguageManager for deprecation and for isCSSPreprocessorFile() to work.

Let me know what you want me to do.

Also, just a thought: what would you say to moving the FilePathUtils to the src/utils folder with the other modules that are classified as utils? it is one of the few utils modules that are not in that folder, and once the old FileSystem code is removed, only FileUtils and FilePathUtils will be in src/file.

[redmunds]

@le717

getSmartFileExtension() requires LanguageManager

It sounds like getSmartFileExtension() belongs in LanguageManager. Does that work?

what would you say to moving the FilePathUtils to the src/utils folder with the other modules that are classified as utils?

Yes, I think that FilePathUtils belongs in the src/utils folder.

[le717]

Moving getSmartFileExtension() to LanguageManager would not effect the temporary circular dependency during deprecation (as it already exists), and would correctly rely on FilePathUtils. The second point would not exist. The only thing that could possibly stop it would be how that one file extension-related is in LanguageManager but all others are in FilePathUtils.

[peterflynn]

This seems more disruptive than it needs to be. Couldn't we also break the circular dependency by just doing this:

• Move getSmartFileExtension() to LanguageManager
• Leave everything else in FileUtils

During the deprecation phase, the cycle would persist of course, but after that it would go away.

The other cleanups seems separable, but could be incorporated here optionally:

• Move _isStatic/ServerHtmlFileExt() to LanguageManager or Language metadata
• Move showFileOpenError() elsewhere to break the circular dependency on Dialogs

It's not the cleanest thing in the world having a mix of utilities that work with paths vs. file contents in one module, but IMHO it's not a nasty enough issue to warrant deprecating over a dozen APIs some of which are probably used in a ton of extensions.

[le717]

@peterflynn

but IMHO it's not a nasty enough issue to warrant deprecating over a dozen APIs some of which are probably used in a ton of extensions.

In the back of my head I've been thinking the same thing, as I've been getting a ton of warnings from this branch with the 20 extensions I've installed, many from very common extensions. A new module might have made sense when the issue was filed, but probably not now.

Move getSmartFileExtension() to LanguageManager
Leave everything else in FileUtils

I'm thinking so, yes, provided isCSSPreprocessorFile() is moved somewhere else at the same time (it too requires LanuageManager).

[peterflynn]

Thinking about it a bit more, here are some possible ideas:

• isCSSPreprocessorFile() -> CSSUtils
• _isStatic/ServerHtmlFileExt() -> LiveDevelopment
  • Possibly change to be based on language id instead of file ext
  • Consider moving the list of server file types into preferences (list of static types should stay hardcoded because it depends on what the Live Preview instrumentation supports). Alternatively, we could add a new flag like isServerPage to Language, and have LiveDevelopment et al check that. This would similarly let users / extension authors customize the list, but make it more of a first-class Language feature. I'm not sure which is best -- it probably depends a bit on our thinking about Live Preview extensibility in the future...
• getSmartFileExtension() -> LanguageManager, and maybe rename to something like getCompoundFileExtension() for clarity
• showFileOpenError() & getFileErrorString() -> ...maybe a new module like FileUI or FileDialogs or ErrorDialogs?

Thoughts?

[le717]

isCSSPreprocessorFile() -> CSSUtils
getSmartFileExtension() -> LanguageManager, and maybe rename to something like getCompoundFileExtension() for clarity

I like these.

showFileOpenError() & getFileErrorString() -> ...maybe a new module like FileUI or FileDialogs or ErrorDialogs?

Perhaps if there are other error dialogs that can be easily relocated, an ErrorDialogs module in src/widgets might work.

_isStatic/ServerHtmlFileExt() -> LiveDevelopment
Possibly change to be based on language id instead of file ext

Since future plans may effect this, I would go for making just these changes and filing a backlog card with extensibility ideas.

[peterflynn]

@le717 Sounds good to me.

Re the dialog, I guess there are actually two options:

• Make a new module, as we've been saying - the only other thing I could think to put in it for now would be the error dialog in DragAndDrop, though (it uses very similar code to makeDialogFileList()).
• Move just showFileOpenError() to DocumentCommandHandlers, next to _showSaveFileError() - DCH is the only non-extension caller of this API anyway. We can leave the more widely-used getFileErrorString() in FileUtils since it doesn't depend on Dialogs.

[le717]

Since making a whole new FilePathUtils module is too drastic to fix the issues, I've spun off the new changes to #10641 and am closing this.