Bug 328444

Comment 1 Thomas Schindl 2010-10-22 05:43:15 EDT

I don't think this is a regression I've take an look at e.g. version 1.60 of TreeViewer and there the same check is in TreeViewer.

I don't think one could have ever set a plain IViewerLabelProvider but had to write code like this:

-------8<-------
public class MyLabelProvider extends LabelProvider implements IViewerLabelProvider {
  // ...
}
-------8<-------

I did not look into the internals so I can't say whether the omission of IViewerLabelProvider was done by intention.

Did you have code that worked in 3.2 but does not in 3.6, which would indicate that impression is wrong?

Comment 2 Nicolas Bros 2010-10-22 06:02:23 EDT

I assumed LabelProvider was only a convenience class, and implementing ILabelProvider or IViewerLabelProvider would be enough.

I'll give you more context: I'm trying to implement a "lazy" label provider, that computes labels in a background Job, and updates the label once the computation is done. For this, I want to avoid synchronous calls to getText, getImage, etc.

I tried extending LabelProvider and implementing IViewerLabelProvider, but ViewerLabel.setText doesn't seem to work in this code. The label stays "loading...":


public class MyLabelProvider extends LabelProvider implements IViewerLabelProvider {
	public void updateLabel(final ViewerLabel label, final Object element) {
		label.setText("loading...");
		Job job = new Job("Update tree element") {
			@Override
			protected IStatus run(final IProgressMonitor monitor) {
				final String text = getText(element);
				Display.getDefault().syncExec(new Runnable() {
					public void run() {
						label.setText(text);
					}
				});
				return Status.OK_STATUS;
			}
		};
                // delay by 3000 ms to test the lazy behavior
		job.schedule(3000);
	}
        ...
}


> Did you have code that worked in 3.2 but does not in 3.6, 
> which would indicate that impression is wrong?
No, I'm trying to use IViewerLabelProvider, and seeing Bug 174355 I thought it could be a regression.

Comment 3 Nicolas Bros 2010-10-22 06:24:19 EDT

I found that the ViewerLabel passed to IViewerLabelProvider is not meant to be used asynchrously, so IViewerLabelProvider is not usable in my case.

But I found that if I used a CellLabelProvider instead, I could update the ViewerCell asynchronously.

Thanks for your help.

I'm marking this bug as fixed, since the problems I had with IViewerLabelProvider seem to come from a misunderstanding on my part.

Comment 4 Thomas Schindl 2010-10-22 06:24:45 EDT

IViewerLabelProvider can't be used this way - the viewerlabel instance is created and passed to the LabelProvider and afterwards it is forgotten.

You need to implement this your own somehow using. E.g.

public class MyLabelProvider extends CellLabelProvider {
   private ColumnViewer viewer;
   private Map<Object,String> labelCache = new HashMap<Object,String>();

   public MyLabelProvider(ColumnViewer viewer) {
     this.viewer = viewer;
   }

   public void update(ViewerCell cell) {
     final Object o = cell.getElement();
     if( labelCache.contains(o) ) {
        cell.setText(labelCache.get(o));
     } else {
        Job job = new Job("Update tree element") {
            @Override
            protected IStatus run(final IProgressMonitor monitor) {
                final String text = getText(element);
                Display.getDefault().syncExec(new Runnable() {
                    public void run() {
                        labelCache.set(o,text);
                        viewer.update(o);
                    }
                });
                return Status.OK_STATUS;
            }
        };
        job.schedule(3000);
     }
   }
}

I'm closing this as works for me because I can't see a problem in our implementation.

Comment 5 Thomas Schindl 2010-10-22 06:31:29 EDT

(In reply to comment #3)
> I found that the ViewerLabel passed to IViewerLabelProvider is not meant to be
> used asynchrously, so IViewerLabelProvider is not usable in my case.
> 
> But I found that if I used a CellLabelProvider instead, I could update the
> ViewerCell asynchronously.
> 

UUUhh - this might give you headaches - you can't cache the ViewerCell passed but need to make a clone. We are reusing the ViewerCell instance

The code to directly work on ViewerCell in an Async way is:

public void update(ViewerCell cell) {
   ViewerCell clone = cell.getViewerRow().getCell(cell.getColumnIndex());
   // ...
}

Comment 6 Nicolas Bros 2010-10-22 09:19:25 EDT

Thanks. Very helpful!