[CLOSED] LESS Refactoring - add LanguageManager

[core-ai-bot]

Issue by DennisKehrig
Monday Feb 11, 2013 at 18:27 GMT
Originally opened as adobe/brackets#2844

---

Submitting this as a pull request so it can get tracked the usual way :)

---

DennisKehrig included the following code: https://github.com/adobe/brackets/pull/2844/commits

[core-ai-bot]

Comment by DennisKehrig
Wednesday Feb 13, 2013 at 15:50 GMT

---

Rebased once more. We should merge this early in a sprint to catch possible mistakes I made when merging.

[core-ai-bot]

Comment by peterflynn
Wednesday Feb 20, 2013 at 06:03 GMT

---

I'm unable to run unit tests from your branch. There are a couple of problems:

• Editor-test has one broken suite that depends on EditorUtils, and it prevents the whole test-runner from loading since it can't find that module. If you fix or comment that out, then you can see the other issues...
• Lots of tests fail -- e.g. about 1/2 of EditorCommandHandlers-test, most of CSSUtils-test, one more test in Editor-test, etc.
• The console gets a big spew of errors about being unable to load various CM modes. I think your change in brackets.js may need to go into SpecRunner.js also. @jasonsanjose probably understands how Require is affected by unit tests the best, though. I'm guessing fixing this will fix a bunch of the test failures too...

[core-ai-bot]

Comment by peterflynn
Wednesday Feb 20, 2013 at 06:04 GMT

---

Done reviewing. Mostly minor changes, I hope, except for the big Languages.js comment and possibly some of the suggestions in DocumentManager...

Great work! I'm very psyched about getting this into Sprint 21...

[core-ai-bot]

Comment by DennisKehrig
Thursday Feb 21, 2013 at 21:58 GMT

---

Rebased on master, fixed the tests and went through all your suggestions.
Thanks for a tremendous job,@peterflynn! You see a lot.

Some of the more important changes:

• Languages.js is now LanguageManager.js
• MIME modes are used like this now: mode: ["javascript", "application/json"]
• Mode aliases are gone
• The "extension" and "mode" plugins for require.js are gone
• There's a language.modeReady promise that resolves when the mode is loaded (true even for already loaded modes)

[core-ai-bot]

Comment by DennisKehrig
Thursday Feb 21, 2013 at 22:01 GMT

---

@jasonsanjose I hope you didn't attempt creating more unit tests yet, it only really makes sense now because I completely forgot to take care of them before today. Sorry!

[core-ai-bot]

Comment by jasonsanjose
Thursday Feb 21, 2013 at 22:11 GMT

---

I didn't start work on unit tests yet. I can review how much more coverage is needed after this pull request lands.

[core-ai-bot]

Comment by jasonsanjose
Friday Feb 22, 2013 at 01:43 GMT

---

Just did my own review on top of@peterflynn's since I was about to tackle unit tests.

[core-ai-bot]

Comment by DennisKehrig
Monday Feb 25, 2013 at 14:54 GMT

---

Hey@jasonsanjose, thanks for the review! I updated the code. There are two things that I commented on that you may not yet be satisfied with.

[core-ai-bot]

Comment by jasonsanjose
Monday Feb 25, 2013 at 20:06 GMT

---

Thanks@DennisKehrig. The latest changes are good, but there were additional comments that weren't addressed yet.

[core-ai-bot]

Comment by jasonsanjose
Tuesday Feb 26, 2013 at 23:23 GMT

---

@peterflynn@DennisKehrig Merged with master, fixed conflict in FileUtils.

[core-ai-bot]

Comment by peterflynn
Tuesday Feb 26, 2013 at 23:48 GMT

---

@jasonsanjose Pushed the updates to fix JS code hints -- turned out to be simple. Want to review my commit real quick?

[core-ai-bot]

Comment by jasonsanjose
Tuesday Feb 26, 2013 at 23:51 GMT

---

JavaScriptCodeHints changes look good

[core-ai-bot]

Comment by peterflynn
Wednesday Feb 27, 2013 at 00:00 GMT

---

@DennisKehrig: There are still a few issues to address here (I think mostly nits) but@jasonsanjose and I agreed it's better to merge this now to get more bake time. Merging now, and then I'll file spinoff bugs assigned to you for the remaining bits.

[core-ai-bot]

Comment by peterflynn
Wednesday Feb 27, 2013 at 00:59 GMT

---

I've spun off all the remaining smaller code review comments into #2968.