| Summary: | [Decorators] Package Explorer misses project nature resource changes | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | [Eclipse Project] Platform | Reporter: | Eike Stepper <stepper> | ||||||
| Component: | UI | Assignee: | Platform UI Triaged <platform-ui-triaged> | ||||||
| Status: | CLOSED WONTFIX | QA Contact: | |||||||
| Severity: | normal | ||||||||
| Priority: | P5 | CC: | dirk_baeumer, dj.houghton, ed.burnette, Ernest.Pasour, n.a.edgar, Tod_Creasey | ||||||
| Version: | 3.1 | Keywords: | helpwanted | ||||||
| Target Milestone: | --- | ||||||||
| Hardware: | PC | ||||||||
| OS: | Windows XP | ||||||||
| Whiteboard: | stalebug | ||||||||
| Attachments: |
|
||||||||
The package explorer doesn't do anything special here. We correctly install the DecoratingLabelProvider with the following label decorator. PlatformUI.getWorkbench().getDecoratorManager().getLabelDecorator() Moving to Platform/UI since the actual decoration is done there. Please advise if there is something JDT/UI has to do to fix this. Created attachment 26403 [details]
Test plug-in
Here is a test plug-in I wrote with my own nature based decorator.
If you open the resource navigator and the package explorer in the same
perspective and select Nature Test -> Toggle nature from the pop-up menu from
the Resource Navigator when a project is selected you will see this decorator
update for both.
Is your decorator adapatable? I am going to close this as WORKSFORME - if you
have a configuration that this is not working for please reopen with more
details.
Marking as WORKSFORME - BTW just noticed yours is adaptable. How are you changing the natures and how is it different from what I did? Tod, please read the last paragraph of my original bug description again. if you have both views open, package explorer *and* navigator, there is no problem. the problem only occurs, if the package explorer is visible *before* the navigator is enabled. Can you confirm this? Created attachment 26446 [details]
Updated test
Here is an updated version of the test that puts the new item in both the
pacakge explorer and resource navigator.
The issue is responding the resource changes. When there is a resource change
the resource navigator requests a label update. The decorator manager then
sends the update to all of it's listeners which include the package explorer.
The issue is that the package explorer does not make a request as a result of
this resource change - it looks like the ResourceToItems mapper needs to be
called from a resource changed listener somewhere.
Moving to JDT and renaming as this is an issue with resource change listening. If you want to see who is posting decoration requests stick a break point in DecoratorManager.prepareDecoration. Tod, why should the package explorer trigger label updates on "random" resource changes. We can listen to nature changes and trigger the update however what if a decorator is based on some other property (for example a plugin state or system property). Are we responsible to know all the properties a decorator might be interested in and trigger the update ? This sounds wrong to me (the package explorer doesn't listen to CVS changes either and triggers the label update. This is done by CVS). IMO either the label decorator infrastrucutre has to trigger this by interpreting the <enablement> section of the decorator extension point or the plug-in that provides the decorator has to provide some code as well which is responsible to trigger the decoration. Moving back to Platform/UI to comment on this. I'm not sure if I understand the concerns about "random" resource changes.
Each time the "property" changes, I fire a ChangedEvent like this:
final LabelProviderChangedEvent event = new LabelProviderChangedEvent(this,
new Object[] {resource});
Display.getDefault().asyncExec(new Runnable()
{
public void run()
{
fireLabelProviderChanged(event);
}
});
Please don't ignore the fact, that all labels are updated correctly, *after* the
navigator is shown the *first time*! For me this is clearly a hint, that
"something" is not working as expected.
Maybe it's also a hint, that I use the folowing method (found in an article on
eclipse.org):
public boolean isLabelProperty(Object arg0, String arg1)
{
return false;
}
And once more: If *only* the Package Explorer is visible (not the Navigator),
the fireLabelProviderChanged() method is not reflected in it. If I restart
Eclipse, the Package Explorer initializes correctly and applies the decorator.
Property changes that are then applied are not reflected until the Navigator is
activated at least a single time. After that the Package Explorer behaves correctly.
The "property change" is detected in another class, that this Decorator listens
to via notifyProductChange().
Here are the relevant sources:
==================================================================================
<extension
point="org.eclipse.ui.decorators">
<decorator
adaptable="true"
icon="icons/full/ovr16/model_ovr.gif"
id="com.sympedia.mde.ui.ModelLabelDecorator"
label="Density MDE Product Models"
lightweight="true"
location="TOP_LEFT"
state="true">
<description>
Decorates Density MDE product models with a model icon.
</description>
<enablement>
<and>
<objectClass name="org.eclipse.core.resources.IProject"/>
<objectState
name="projectNature"
value="com.sympedia.mde.mcore.ModelNature"/>
</and>
</enablement>
</decorator>
</extension>
==================================================================================
package com.sympedia.mde.ui.internal.decorators;
import com.sympedia.density.util.ImplementationError;
import com.sympedia.density.util.eclipse.ResourcesHelper;
import com.sympedia.mde.mcore.IProductListener;
import com.sympedia.mde.mcore.Mcore;
import com.sympedia.mde.mcore.Product;
import com.sympedia.mde.mcore.ProductType;
import com.sympedia.mde.mcore.Root;
import com.sympedia.mde.ui.internal.MdeUiImages;
import com.sympedia.mde.ui.internal.MdeUiPlugin;
import org.apache.log4j.Logger;
import org.eclipse.core.resources.IResource;
import org.eclipse.core.runtime.IAdaptable;
import org.eclipse.core.runtime.IPath;
import org.eclipse.jface.viewers.IDecoration;
import org.eclipse.jface.viewers.ILightweightLabelDecorator;
import org.eclipse.jface.viewers.LabelProvider;
import org.eclipse.jface.viewers.LabelProviderChangedEvent;
import org.eclipse.swt.widgets.Display;
public class ModelFolderLabelDecorator extends LabelProvider implements
ILightweightLabelDecorator,
IProductListener
{
private static final Logger logger =
Logger.getLogger(ModelFolderLabelDecorator.class);
private static final ProductType MODEL_FOLDER_TYPE =
Mcore.INSTANCE.MODEL_FOLDER_TYPE();
private static final ProductType MODEL_PACKAGE_TYPE =
Mcore.INSTANCE.MODEL_PACKAGE_TYPE();
private static ModelFolderLabelDecorator SINGLETON_GUARD;
public ModelFolderLabelDecorator()
{
if (SINGLETON_GUARD != null)
{
throw new ImplementationError("SINGLETON GUARD");
}
Root.INSTANCE.addProductListener(this);
SINGLETON_GUARD = this;
}
public void decorate(Object object, IDecoration decoration)
{
try
{
IResource resource = getResource(object);
if (resource == null || resource.getType() != IResource.FOLDER)
{
return;
}
Product product = Root.INSTANCE.getProduct(resource.getFullPath());
if (product != null)
{
ProductType productType = product.getProductType();
if (productType == MODEL_FOLDER_TYPE || productType == MODEL_PACKAGE_TYPE)
{
decoration.addOverlay(MdeUiImages.DESC_OVR_MODEL);
}
}
}
catch (Exception ex)
{
MdeUiPlugin.getDefault().error("Problem while decorating", ex);
}
}
public void dispose()
{
Root.INSTANCE.removeProductListener(this);
SINGLETON_GUARD = null;
}
public boolean isLabelProperty(Object arg0, String arg1)
{
return false;
}
public void notifyProductChange(IPath fullPath, String productTypeName)
{
IResource resource = ResourcesHelper.WS_ROOT.findMember(fullPath);
if (resource != null)
{
if (logger.isDebugEnabled())
{
logger.debug("Product change: " + fullPath + " type=" + productTypeName);
}
fireLabelEvent(resource);
}
}
private void fireLabelEvent(IResource resource)
{
final LabelProviderChangedEvent event = new LabelProviderChangedEvent(this,
new Object[] {resource});
Display.getDefault().asyncExec(new Runnable()
{
public void run()
{
fireLabelProviderChanged(event);
}
});
}
private IResource getResource(Object object)
{
if (object instanceof IResource)
{
return (IResource)object;
}
if (object instanceof IAdaptable)
{
return (IResource)((IAdaptable)object).getAdapter(IResource.class);
}
return null;
}
}
Sorry, I've cut the wrong markup from plugin.xml, here's the correct:
</decorator>
<decorator
adaptable="true"
label="Density MDE Model Folders"
state="true"
lightweight="true"
class="com.sympedia.mde.ui.internal.decorators.ModelFolderLabelDecorator"
id="com.sympedia.mde.ui.ModelFolderLabelDecorator"
location="TOP_LEFT">
<description>
Decorates Density MDE model folders with a model icon.
</description>
<enablement>
<objectClass name="org.eclipse.core.resources.IFolder"/>
</enablement>
</decorator>
Forgive me, this backward and forward on the bugzilla totally confused me and my
many decorators ;-)
PLEASE IGNORE MY COMMENTS #8 to #10!!!
The truth is the following:
I use a LightWeightLabelDecorator without any additional class. It shall only
respond to nature changes of IProjects:
<decorator
adaptable="true"
icon="icons/full/ovr16/model_ovr.gif"
id="com.sympedia.mde.ui.ModelLabelDecorator"
label="Density MDE Product Models"
lightweight="true"
location="TOP_LEFT"
state="true">
<description>
Decorates Density MDE product models with a model icon.
</description>
<enablement>
<and>
<objectClass name="org.eclipse.core.resources.IProject"/>
<objectState
name="projectNature"
value="com.sympedia.mde.mcore.ModelNature"/>
</and>
</enablement>
</decorator>
My original observations with this Decorator still hold valid!
No one is questioning your analysis Eike - you are spot on. The issue for JDT Dirk is that there is an update from Core that you do not refresh your label provider for this event. I'm not sure what you think the UI needs to do here are we are out of the equation. Natures are one of the enablements for objectContributions so this might be an issue for some other things for you too. Tod,
I am not sure if I understand your last comment correctly but here is my current
understanding:
there is a label decorator support which is fully declarative (no code envolved
so the plug-in that declares the decorator can't listen to the delta and trigger
the label decorator updates). However the decorators should be updated when
their enablement state changes, but this state can be fairly complex
(objectState, plug-in state). Consider a label decorator that decorates all
projects when a certain plug-in gets activated.
<extension
point="org.eclipse.ui.decorators">
<decorator
adaptable="true"
icon="icons/binary_co.gif"
id="ContentTypeTest.decorator2"
label="ContentTypeTest.decorator2"
location="TOP_LEFT">
<enablement>
<and>
<objectClass name="org.eclipse.core.resources.IProject"/>
<pluginState
id="org.eclipse.jdt.debug.ui"
value="activated"/>
</and>
</enablement>
</decorator>
</extension>
Who is responsible to trigger this label decorator update. When I activate the
debug plug-in neither the Navigator nor the Package explorer shows any
decorations afterwards. So IMO triggering the label decorator update shouldn't
be the viewer's responsibility. Otherwise they have to listen to bundle changes.
IMO Platform/UI is in the "equation" since the plug-in provides the declarative
label decorator so it should tirgger the correspondings updates as well (however
I agree that this is a fairly complicated thing to do). Otherwise all viewers
have to be aware of this even though they don't know that they are decorated.
The package explorer currently doesn't react to project description changes
since from a pure package explorer point of view there is no need to do so. No
visual structure changed for it. I looked into reacting to project description
changes and it is doable however this will only fix this case. If a decorator is
enabled for a different state we might fail again.
OK I understand your point but as you saying that anything that uses object contributions should listen for changes that require an update? The decorator manager is currently a passive object so I am not sure if it is a good idea to add it as a resource changed listener as it might duplicate a lot of work for the othe projects that are resource changed listeners. I think this is an interesting meta-question as having a general solution might simply the implementation of viewers in general. Adding Nick to get his opinion. Dirk I think the issue here needs to be how your content provider handles updates. I have logged Bug 108128 to handle the case you are describing in the WorkbenchContentProvider. If I am not mistaken you are not using the WorkbenchContentProvider so you will have to implement the update mechanism yourself. In the end I think this is a problem for a Generic navigator as it is clearly something that anyone would have trouble with if they do not use the WorkbenchContentProvider Tod, I don't think that this should be handled in the WorkbenchContentProvider since as you said views will very likely not use it. I will add more comments to bug 108128 After digging around in the resource changes it will not be possible for a declarative decorator to update a session or persistant property everytime it changes as there is no resource changed event sent by a property change for performance reasons. The implementor of the decorator will have to trigger updates instead using IDecoratorManger#update. A property description change like a nature change is something we may be able to do something about so I will continue to investigate. I have updated the schema for the decorators to reflect this issue with properties in build >20050909 Marking as WORKSFORME as it looks like I have given you all you need 3.2RC1
Do you mind if I reopen this one? While the workaround works, it has two problems:
1. It creates an unnecessary coupling between the project nature and the decorator. The nature should not have to be aware of any decorators or other things that are enabled by it.
2. Calling IDecoratorManager.update() causes the decorators on all projects to flash (disappear and reappear).
You can see the problem if you create a plug-in and use the template for a plug-in project with natures. Then add the extension template that does lightweight decorations, then change the enablement to depend on the nature. Toggling the nature ON will show the decoration, but toggling it OFF will not remove the decoration. You have to close the project and reopen it to remove the decoration.
To fix this with the workaround, in ToggleNatureAction.toggleNature() you have to add a line that does:
PlatformUI.getWorkbench().getDecoratorManager().update("xx");
where "xx" is the name of your declarative decorator. If you do that it works but has the problems above.
There are currently no plans to work on this although we would be happy to review a patch Oleg is now responsible for watching the [Decorators] category. This bug hasn't had any activity in quite some time. Maybe the problem got resolved, was a duplicate of something else, or became less pressing for some reason - or maybe it's still relevant but just hasn't been looked at yet. If you have further information on the current state of the bug, please add it. The information can be, for example, that the problem still occurs, that you still want the feature, that more information is needed, or that the bug is (for whatever reason) no longer relevant. This bug hasn't had any activity in quite some time. Maybe the problem got resolved, was a duplicate of something else, or became less pressing for some reason - or maybe it's still relevant but just hasn't been looked at yet. As such, we're closing this bug. If you have further information on the current state of the bug, please add it and reopen this bug. The information can be, for example, that the problem still occurs, that you still want the feature, that more information is needed, or that the bug is (for whatever reason) no longer relevant. -- The automated Eclipse Genie. |
Please consider the following code-less decorator: <decorator adaptable="true" icon="icons/full/obj16/system_ovl.gif" id="com.sympedia.mde.product.ui.SystemLabelDecorator" label="MDE System" lightweight="true" location="TOP_LEFT" state="true"> <description> Decoration of MDE Systems </description> <enablement> <and> <objectClass name="org.eclipse.core.resources.IProject"/> <objectState name="projectNature" value="com.sympedia.mde.product.systemnature"/> </and> </enablement> </decorator> Adding and removing the nature doesn't affect the icon in the Package Explorer. Only the Package Explorer seems to have this issue. The Navigator does it well. Even more surprising: After switching from Package Explorer to Navigator only once, and back, the Package Explorer behaves correctly from then on.