Page MenuHomePhabricator

ve.ui.MWAceEditorWidget breaks autoloading of text modes
Closed, ResolvedPublic1 Estimated Story Points

Description

// TODO: Just use ace.require once T127643 is resolved
var require = ace.require || require;
widget.editor.getSession().setMode( 'ace/mode/' + ( require( 'ace/mode/' + lang ) ? lang : 'text' ) );

We are being too smart for our own good here...

Now that we have set the basePath for Ace, we can use auto loading, but the above strategy doesn't work with that methodology. The reason is that require, needs to have a module define itself first. Therefore, the module needs to be loading before code requires it. Since only few modes of the total amount of modes are preloaded by RL (ext.codeEditor.ace and ext.codeEditor.ace.modes), we are getting less than we could (because requiring an undefined module returns undefined).

Instead just use setMode directly and let ACE handle everything, including loading the resource so that it can define itself, requiring, and the setting of the language. It will also fallback to ace/mode/text on it's own.

widget.editor.getSession().setMode( 'ace/mode/' + lang );

If this is fixed, we probably don't need to load ext.codeEditor.ace.modes in the widget any longer either (or at all actually).

Event Timeline

Change 316572 had a related patch set uploaded (by TheDJ):
Enable conditional loading of ACE language modes

https://gerrit.wikimedia.org/r/316572

Change 316572 merged by jenkins-bot:
Enable conditional loading of ACE language modes

https://gerrit.wikimedia.org/r/316572

TheDJ claimed this task.
TheDJ triaged this task as Low priority.
TheDJ removed a project: Patch-For-Review.
Jdforrester-WMF set the point value for this task to 1.