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

Remaining code cleanups for language APIs #2968

Description

@peterflynn

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 $.each() -- we've been moving away from $.each() because it's buggy if arbitrary string keys are possible.
  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)

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Type

No type

Projects

No projects

    Milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions