Bugzilla – Full Text Bug Listing
| Summary: | Syntax highlighter services are being registered multiple times by incorrect plug-ins | ||||||
|---|---|---|---|---|---|---|---|
| Product: | [ECD] Orion | Reporter: | Curtis Windatt <curtis.windatt.public> | ||||
| Component: | Editor | Assignee: | Curtis Windatt <curtis.windatt.public> | ||||
| Status: | RESOLVED FIXED | QA Contact: | |||||
| Severity: | major | ||||||
| Priority: | P3 | CC: | grant_gayed, mamacdon, Michael_Rennie | ||||
| Version: | 6.0 | ||||||
| Target Milestone: | 6.0 RC1 | ||||||
| Hardware: | PC | ||||||
| OS: | Windows 7 | ||||||
| Whiteboard: | |||||||
| Attachments: |
|
||||||
We can workaround this by only registering a specific grammar id in the plug-ins
for (var i=mCSS.grammars.length-1; i>=0; i--) {
if (mCSS.grammars[i].id === "orion.css"){
provider.registerServiceProvider("orion.edit.highlighter", {}, mCSS.grammars[i]);
break;
}
}
for (var i=mHTML.grammars.length-1; i>=0; i--) {
if (mHTML.grammars[i].id === "orion.html"){
provider.registerServiceProvider("orion.edit.highlighter", {}, mHTML.grammars[i]);
break;
}
}
(In reply to Curtis Windatt from comment #1) > We can workaround this by only registering a specific grammar id in the > plug-ins > For fun I made a patch for the JS bundle doing the same thing of only registering the grammars we actually care about (orion.js, orion,json, orion.schema.json and orion.ejs) and profiled my host and target doing a non-cached page load of the same JS file to measure how long it takes to load javaScriptPlugin.js: 1. registering all of the grammars took 185ms, 308 requests, 686ms complete refresh time 2. registering only the ones we care about took 46ms, 302 requests, 510ms complete refresh time Making sure we only register ones relevant to JS also drops the service count on the JS bundle from 51 to 18. http://git.eclipse.org/c/orion/org.eclipse.orion.client.git/commit/?id=b1304d0188c093944e0a8f0cc966bdc72a185e31 Still no additional info on why the grammars array includes everything, so for now I have fixed the javascript plug-in. Not only will it improve performance, it is a lot easier to look through 18 services vs 51 services. I will leave this bug open as fixing the editor code would be better. Created attachment 243592 [details] screenshot I just cleared my local storage, and now most of the JavaScript syntax highlighting is missing. (see pic) If I revert the commit from Comment 3 (and clear localStorage again) the highlighting comes back. Curtis, can you look into this? Perhaps the redundant registration of every grammar again and again was being relied on in order for references to shared syntax rules (eg. for block comments and string literals) to be resolved. Grant should know. (In reply to Mark Macdonald from comment #4) > Created attachment 243592 [details] > screenshot > > I just cleared my local storage, and now most of the JavaScript syntax > highlighting is missing. (see pic) If I revert the commit from Comment 3 > (and clear localStorage again) the highlighting comes back. > > Curtis, can you look into this? Perhaps the redundant registration of every > grammar again and again was being relied on in order for references to > shared syntax rules (eg. for block comments and string literals) to be > resolved. Grant should know. I see this now, there is still keyword highlighting, but not full js highlighting (comments, etc.). (In reply to Mark Macdonald from comment #4) > Curtis, can you look into this? Perhaps the redundant registration of every > grammar again and again was being relied on in order for references to > shared syntax rules (eg. for block comments and string literals) to be > resolved. Grant should know. Your guess appears to be correct, the js grammar includes items from orion.lib and orion.c-like, which are no longer available. For now I will revert my change. Hopefully we can come up with a better solution than re-registering everything on the stack. Thanks Curtis. I would like to find a solution that doesn't require re-declaring the same grammars also. It should work to take all of the current grammars and remove the duplicates (based on each grammar's id) before registering them. (In reply to Grant Gayed from comment #9) > It should work to take all of the current grammars and remove the duplicates > (based on each grammar's id) before registering them. http://git.eclipse.org/c/orion/org.eclipse.orion.client.git/commit/?id=2e3e60d9abdfb55270e0e2b5d7ca683482bb033f This change maps the grammars by id so each plug-in registers each grammar only once. This still doesn't seem optimal as anytime a plug-in is loaded, it has to register the same grammars (c-like, lib, etc). It would be better if we could determine which grammars have already been registered (when the plug-in loads). Note that all of the other syntax highlighters only register their single syntax.js file (which works because the client ui plug-in loads them all). No further changes planned for RC1. Any additional improvements will require changes to how the grammars are created and stored which isn't a good idea for RC1. |
When creating a new bundle, I am having problems registering syntax highlighting for multiple languages. The grammars array returned by the various syntax.js files in the editor bundle always includes all previously loaded syntax grammars. Most plug-ins only register the last grammar in the list, which was (always?) the correct one. i.e. the previous cssPlugin.js did the following: provider.registerServiceProvider("orion.edit.highlighter", {}, mCSS.grammars[mCSS.grammars.length - 1]); The last grammer in the list would be orion.css. However, when trying to do the same from a new plug-in: provider.registerServiceProvider("orion.edit.highlighter", {}, mCSS.grammars[mCSS.grammars.length - 1]); provider.registerServiceProvider("orion.edit.highlighter", {}, mHTML.grammars[mHTML.grammars.length - 1]); The results are inconsistent and wrong. Often the last entry in the grammars array will be orion.js (sometimes orion.xml or others). This results in the css syntax highlighter never being registered. I also saw that the javascript plugin does it differently yet: var grammars = mJS.grammars.concat(mJSON.grammars).concat(mJSONSchema.grammars).concat(mEJS.grammars); grammars.forEach(function(current) { provider.registerService("orion.edit.highlighter", {}, current); }.bind(this)); As the whole array is registered, the javascript plug-in is registering many highlighters it does not provide (orion.lib, orion.c-like, orion.xml, etc.). Worse, the list is concatenated multiple times, so the same service is registered three times. The Javascript plug-in current says it has 51 services!