[CLOSED] Remaining code cleanups for language APIs #2800
Description
Wednesday Feb 27, 2013 at 00:58 GMT
Originally opened as adobe/brackets#2968
This is a continuation of the code review in #2844. The branch was landed before some smaller issues were addressed to give it additional bake time.
Remaining issues:
1.@jasonsanjose and both felt that mode shouldn't be optional when calling defineLanguage(). It seems virtually guaranteed to be a bug in the extension, so best to fail loudly instead of silently. To handle the rare case where a developer wants to add limited language support before code coloring is available, the developer could just explicitly specify "text/plain" as the mode. (See this comment and to some degree this one)
2. Language's constructor allows id to contain "." but not "_", the opposite of the docs. Though I actually like "." better since it's more consistent with our other id naming conventions, and I don't really see the need for explicit restrictions anyway... (See this comment).
3. Language._addFileExtension() may want to check for a "." in the string since it'd be an easy mistake to make. Or we could handle both like getLanguageForFileExtension() does.
4. The iteration over _defaultLanguagesJSON should use CollectionUtils.forEach() instead of
5. I think Language._setMode() can just use Array.isArray() instead of $.isArray()
6. The way LanguageManager sets its exports at the end doesn't match our usual format
7. Docs on the defineMIME("text/x-brackets-html"... call could use clarification (see this comment)
8. Language doesn't have JSDocs (or a prototype declaration) for these fields: mode, blockComment, lineComment, _fileExtensions, _modeToLanguageMap, _modeReady
* Should blockComment, lineComment be private though? They have a setter you're supposed to use...
9. JSDocs in Editor still reference Languages instead of LanguageManager
10. Nit: Seems like _fileExtensionsToLanguageMap should be named _fileExtensionToLanguageMap (without the "s") -- more consistent with how we name other maps (the key is not plural)