Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.
Bug 313958 - Make renderer factory configurable
Summary: Make renderer factory configurable
Status: RESOLVED FIXED
Alias: None
Product: e4
Classification: Eclipse Project
Component: UI (show other bugs)
Version: 1.0   Edit
Hardware: PC Mac OS X - Carbon (unsup.)
: P3 normal (vote)
Target Milestone: 1.0 RC0   Edit
Assignee: Project Inbox CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-05-21 14:16 EDT by Thomas Schindl CLA
Modified: 2010-06-09 19:29 EDT (History)
7 users (show)

See Also:

Attachments
Patch (8.67 KB, text/plain)
2010-05-21 14:16 EDT, Thomas Schindl CLA 		no flags Details
Updated Patch (7.90 KB, patch)
2010-05-21 14:21 EDT, Thomas Schindl CLA 		no flags Details | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Thomas Schindl CLA 2010-05-21 14:16:44 EDT 
Created attachment 169537 [details]
Patch

We should use DI to create the IRendererFactory (this would remove the need for the init()-method in the interface) and allow the user/developer to configure it when creating an application and/or launching.

The patch attached implements this and allows the user/developer to customize it passing the uri to the renderer factory
Comment 1 Thomas Schindl CLA 2010-05-21 14:18:49 EDT 
This patch makes the extension point and the contribution bundle need less.
Comment 2 Thomas Schindl CLA 2010-05-21 14:21:28 EDT 
Created attachment 169538 [details]
Updated Patch
Comment 3 Thomas Schindl CLA 2010-05-22 11:45:49 EDT 
Yves, I think I saw that you are setting your own RendererFactory in the Desginer code. Once I'll release this change you need to react on it.
Comment 4 Thomas Schindl CLA 2010-05-25 13:16:24 EDT 
released changes to CVS
Comment 5 Yves YANG CLA 2010-05-25 22:52:02 EDT 
There is an error in the constructor of the class PartRenderingEngine:

	@Inject
	public PartRenderingEngine(
			@Named(E4Workbench.RENDERER_FACTORY_URI) @Optional String factoryUrl) {
		if (factoryUrl == null) {
			factoryUrl = defaultFactoryUrl;
		}
		this.factoryUrl = defaultFactoryUrl;
	}

The this.factoryUrl is always initialized by defaultFactoryUrl. The argument is never used. I think the correct code should be:

	@Inject
	public PartRenderingEngine(
			@Named(E4Workbench.RENDERER_FACTORY_URI) @Optional String factoryUrl) {
		if (factoryUrl == null) {
			factoryUrl = defaultFactoryUrl;
		}
		this.factoryUrl = factoryUrl;
	}

I'll commit this correction with my update of Visual Design Editor.
Comment 6 Yves YANG CLA 2010-05-25 22:53:27 EDT 
VDE is updated.
Comment 7 Remy Suen CLA 2010-05-26 08:07:31 EDT 
Thanks for catching this, Yves!
Comment 8 Lars Vogel CLA 2010-06-09 18:55:39 EDT 
factoryUrl is null, even if I specify the property "rendererFactoryUri".

For an easy test you can set the rendererFactoryUri to the standard factory and see that the factoryUrl is still null in PartRenderingEngine. I tried to debug this but the DI framework did not reveal it secrets to me. 


This is the property:

<property
      name="rendererFactoryUri"
      value="platform:/plugin/org.eclipse.e4.ui.workbench.renderers.swt/org.eclipse.e4.ui.workbench.renderers.swt.WorkbenchRendererFactory">
 </property>


This is the part to check:


public PartRenderingEngine(
			@Named(E4Workbench.RENDERER_FACTORY_URI) @Optional String factoryUrl) {

if (factoryUrl == null) {
			factoryUrl = defaultFactoryUrl;
		}
Comment 9 Thomas Schindl CLA 2010-06-09 19:13:06 EDT 
Looks like we forgot to set it into the context - fixed setting the url into the root-context when starting up.
Comment 10 Lars Vogel CLA 2010-06-09 19:15:49 EDT 
Thanks Tom, that was fast. Which plugin do I need to update to check (my internet connection is extremely slow at the moment)?
Comment 11 Thomas Schindl CLA 2010-06-09 19:17:03 EDT 
org.eclipse.e4.ui.workbench.swt
Comment 12 Lars Vogel CLA 2010-06-09 19:25:41 EDT 
Sorry, but after an update of org.eclipse.e4.ui.workbench.swt from cvs
factoryUrl is still null.
Comment 13 Lars Vogel CLA 2010-06-09 19:29:02 EDT 
Sorry, my error. Fixed with the change in EApplication.java.

Thanks again Tom.