Continued Language API work #2979

Closed
wants to merge 6 commits into
from

Conversation

Projects
None yet
3 participants
Contributor

DennisKehrig commented Feb 27, 2013

Focuses on #2968 - minus its comments, so far

DennisKehrig added some commits Feb 27, 2013

When defining a language, setting a mode is now required. In addition…
…, defineLanguage now returns a promise that resolves once the mode has been loaded and set, instead of returning the language directly. To make things more consistent both within LanguageManager and with the rest of the code base, chaining support has been removed.
Language now encapsulates its properties and makes them accessible ex…
…clusively through getters and setters

Adjust the unit tests accordingly
Added documentation to Language's properties
Refined the comment about our special HTML MIME mode
Renamed fileExtensionsToLanguageMap to fileExtensionToLanguageMap
Documentation cleanup
Call result.reject in a few places instead of throwing exceptions
@@ -207,7 +208,7 @@ define(function (require, exports, module) {
// Mode for range - mix of modes
myEditor.setSelection({line: 2, ch: 4}, {line: 3, ch: 7});
- expectModeAndLang(myEditor, null);
+ expectModeAndLang(myEditor, langNames.unknown);
@DennisKehrig

DennisKehrig Feb 27, 2013

Contributor

The intention may have been to specify that the language should also be null if the mode is null, but so far we return the fallback language to always return a language.

+ }
+ }
+
+ return { promise: promise, language: language };
@DennisKehrig

DennisKehrig Feb 27, 2013

Contributor

For convenience, return both a promise and a language.
Maybe this is a good idea for LanguageManager.defineLanguage(), too?
Or maybe we want to go in the opposite direction, i.e. make sure that a language is only registered (via ID, mode or file extensions) once it's fully specified. I.e. we'd wait with registering it until just before we resolve the promise in defineLanguage to make this basically an atomic operation and not have "incomplete" languages lying around.

+
+ runs(function () {
+ validateLanguage(def, language);
+ });
});
@DennisKehrig

DennisKehrig Feb 27, 2013

Contributor

Not sure if this test is specified correctly, please check closely if you are more familiar with Jasmine than I am.

- validateLanguage(def, lang);
+ waitsFor(function () {
+ return Boolean(language);
+ }, "The language should be resolved", 50);
@DennisKehrig

DennisKehrig Feb 27, 2013

Contributor

Very short timeout since this should work synchronously when no mode has to be loaded.

Contributor

DennisKehrig commented Feb 27, 2013

@jasonsanjose I updated the unit tests, too, in case you want to double check.

@ghost ghost assigned jasonsanjose Feb 27, 2013

Member

jasonsanjose commented Feb 27, 2013

Oops. Looks like that overlaps with #2978. I'll reconcile your branch after that lands.

Member

peterflynn commented Feb 27, 2013

@jasonsanjose Should we close this now that #2980 is up?

Member

jasonsanjose commented Feb 27, 2013

Closing, see #2980

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