Bug 484105 - Editor and file-association registry to listen to Extension Registry changes
Summary: Editor and file-association registry to listen to Extension Registry changes
Status: CLOSED WORKSFORME
Alias: None
Product: Platform
Classification: Eclipse Project
Component: UI (show other bugs)
Version: 4.5   Edit
Hardware: All All
: P3 enhancement (vote)
Target Milestone: 4.6 M5   Edit
Assignee: Platform-UI-Inbox CLA
QA Contact:
URL:
Whiteboard: RHT
Keywords: helpwanted
Depends on:
Blocks:
 
Reported: 2015-12-10 05:53 EST by Mickael Istria CLA
Modified: 2015-12-24 09:45 EST (History)
1 user (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Mickael Istria CLA 2015-12-10 05:53:22 EST
When installing a new editor, it seems awkward that this is not made available before a restart. My first guess is that the editor registry and/or the file assocation registry are populated only once and do not listen to change on the extension registry.
If it is so, we could add listeners on the extension registry to re-populate the editors/associations registry when something new is installed.
Comment 1 Mickael Istria CLA 2015-12-18 09:57:22 EST
It seems to me that a good implementation would be to add listeners on extension registry into the RegistryReader class, which is used in multiple parts of the Platform. If we can have that, then it means that most extension point could support addition/removal of extensions without having to restart IDE.
Comment 2 Mickael Istria CLA 2015-12-18 11:38:52 EST
A strategy I just tried is to add a Listener on the extension registry from RegistryReader (EditorRegistryReader inherits from it and I could see the listener actually added to the registry). Then I installed a small plugin containing a new editor ( p2 repo http://dadacoalition.org/yedit ) and the listener doesn't receive any event coming from these additions.
I have the impression that the p2 install simply doesn't install (in the OSGi meaning of install) the new bundles into the running IDE. Is it so? If yes, is this something that could be improved? Is there a bug tracking that?
Comment 3 Eclipse Genie CLA 2015-12-22 03:58:16 EST
New Gerrit change created: https://git.eclipse.org/r/63126
Comment 4 Mickael Istria CLA 2015-12-22 04:05:58 EST
With https://git.eclipse.org/r/63126 , adding and OSGi-installing an editor (via Equinox Configurator.applyConfiguration) updates the editor registry and file-association registry so the editor can be open just after an installation without a restart.
IMO, this change is good to go as-it into Platform UI. It allows extenders who know what they're doing to allow installations without restart for editors, at their own risk; without changing anything in "regular" Eclipse IDE for which nothing will happen as there is no Configurator.applyConfiguration() calls.

I made the tests using the Yaml editor from Marketplace https://github.com/oyse/yedit, adding the bundle in the bundle.info and then triggering Configurator.applyConfigurator() after addition on my test instance.
It worked quite well: after the call to Configurator.applyConfiguration(), the Yaml editor opens inside Eclipse IDE and seems normally usable.
However, the log shows for example that some other extensions it's using are not dynamic and trying to consume them produces a stacktrace. The only one I see so far is about keybinding scopes:
org.eclipse.core.commands.common.NotDefinedException: Cannot get the parent identifier from an undefined context. org.dadacoalition.yedit.yeditScope
	at org.eclipse.core.commands.contexts.Context.getParentId(Context.java:194)
	at org.eclipse.e4.ui.bindings.internal.ContextSet$CComp.getLevel(ContextSet.java:49)
	at org.eclipse.e4.ui.bindings.internal.ContextSet$CComp.compare(ContextSet.java:38)
	at org.eclipse.e4.ui.bindings.internal.ContextSet$CComp.compare(ContextSet.java:1)
	at java.util.TimSort.binarySort(TimSort.java:296)
	at java.util.TimSort.sort(TimSort.java:221)
	at java.util.Arrays.sort(Arrays.java:1512)
	at java.util.ArrayList.sort(ArrayList.java:1454)
	at java.util.Collections.sort(Collections.java:175)
	at org.eclipse.e4.ui.bindings.internal.ContextSet.<init>(ContextSet.java:77)
	at org.eclipse.e4.ui.bindings.internal.BindingTableManager.createContextSet(BindingTableManager.java:90)
	at org.eclipse.e4.ui.bindings.internal.BindingServiceImpl.setContextIds(BindingServiceImpl.java:177)
	at sun.reflect.GeneratedMethodAccessor18.invoke(Unknown Source)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.lang.reflect.Method.invoke(Method.java:497)
	at org.eclipse.e4.core.internal.di.MethodRequestor.execute(MethodRequestor.java:56)
	at org.eclipse.e4.core.internal.contexts.ContextObjectSupplier$ContextInjectionListener.update(ContextObjectSupplier.java:90)
	at org.eclipse.e4.core.internal.contexts.TrackableComputationExt.update(TrackableComputationExt.java:111)
	at org.eclipse.e4.core.internal.contexts.EclipseContext.processScheduled(EclipseContext.java:341)
	at org.eclipse.e4.core.internal.contexts.EclipseContext.set(EclipseContext.java:356)
	at org.eclipse.e4.core.internal.contexts.EclipseContext.activate(EclipseContext.java:666)
	at org.eclipse.e4.core.internal.contexts.EclipseContext.activateBranch(EclipseContext.java:672)
	at org.eclipse.e4.ui.internal.workbench.PartActivationHistory.activate(PartActivationHistory.java:53)
	at org.eclipse.e4.ui.internal.workbench.PartServiceImpl.activate(PartServiceImpl.java:727)
	at org.eclipse.e4.ui.internal.workbench.PartServiceImpl.activate(PartServiceImpl.java:662)
	at org.eclipse.e4.ui.internal.workbench.PartServiceImpl.activate(PartServiceImpl.java:657)
	at org.eclipse.ui.internal.WorkbenchPage.busyOpenEditor(WorkbenchPage.java:3264)
	at org.eclipse.ui.internal.WorkbenchPage.access$25(WorkbenchPage.java:3171)
	at org.eclipse.ui.internal.WorkbenchPage$10.run(WorkbenchPage.java:3153)
	at org.eclipse.swt.custom.BusyIndicator.showWhile(BusyIndicator.java:70)
	at org.eclipse.ui.internal.WorkbenchPage.openEditor(WorkbenchPage.java:3148)
	at org.eclipse.ui.internal.WorkbenchPage.openEditor(WorkbenchPage.java:3112)
	at org.eclipse.ui.internal.WorkbenchPage.openEditor(WorkbenchPage.java:3102)
	at org.eclipse.ui.ide.IDE.openEditor(IDE.java:555)
Comment 5 Mickael Istria CLA 2015-12-24 09:45:52 EST
Actually, the various (disabled) tests in DynamicPluginsTestSuite have raised my awarness on the work done years ago by Kim (and Pascal?). Those tests seem to be failing for various reasons: they're using the pre-OSGi extension which doesn't seem to be supported any more, there are some race conditions on the way tests are written.
But, I did verify the behaviour and was very positively surprised to verify that the initial implementation is still working for the user-story reported here.
Without changing anything, here are some nice scenario:
* Start an Eclipse IDE
* Have some yaml files in the workspace
* Open yaml file => System editor shows up
* Add a line such as "org.dadacoalition.yedit,1.0.20.201509041456-RELEASE,file:/home/mistria/Downloads/org.dadacoalition.yedit_1.0.20.201509041456-RELEASE.jar,4,false" in configuration/org.eclipse.equinox.simpleconfigurator/bundles.info
* Find a way to trigger Configurator.applyConfigurator() - I've created a dummy menu with a handler for that
* Open yaml file => Yaml editor shows up (which highlight that editor and file-association registry are updated on installation change)
* Remove the line from bundles.info
* trigger Configurator.applyConfigurator() again
* Open Yaml file -> Text editor shows up - highlighing that the editor and file-association registry are also listening to removal

So I believe this was a false alarm and that nothing is required here. On a posistive note, it was an opportunity to remind that for many extensions of Platform UI, we can assume that those are dynamic enough to avoid some restarts.
Also, if we're going to rely on it more and more, it would be interesting to revive the DynamincPluginsTestSuite.
I'll use this tested behavior and proposal for bug 90292 to draft the IDE-side part of bug 480176