Bug 156277 - [TabbedProperties] Tabbed Properties support filters and typeMapping for ITextSelection
Summary: [TabbedProperties] Tabbed Properties support filters and typeMapping for ITex...
Status: RESOLVED WORKSFORME
Alias: None
Product: Platform
Classification: Eclipse Project
Component: UI (show other bugs)
Version: 3.3   Edit
Hardware: PC Windows XP
: P3 enhancement with 2 votes (vote)
Target Milestone: ---   Edit
Assignee: Anthony Hunter CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks: 154781
  Show dependency tree
 
Reported: 2006-09-05 17:50 EDT by Karen Butzke CLA
Modified: 2008-06-20 11:55 EDT (History)
4 users (show)

See Also:


Attachments
new extension point attribute for providing an IStructuredSelection (15.45 KB, patch)
2006-09-07 16:04 EDT, Karen Butzke CLA
no flags Details | Diff
patch to add ISelectionConverter as an option for property contributors (15.91 KB, patch)
2006-10-17 10:19 EDT, Karen Butzke CLA
no flags Details | Diff
adds ISelectionConverter as an option for property contributors (15.91 KB, patch)
2006-10-17 10:22 EDT, Karen Butzke CLA
no flags Details | Diff
New patch without current api changes (8.54 KB, patch)
2006-11-13 11:51 EST, Karen Butzke CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Karen Butzke CLA 2006-09-05 17:50:01 EDT
In TabbedPropertyRegistryClassSectionFilter.appliesToSelection(ISectionDescriptor, ISelection) only IStructuredSelections are considered.  I believe that ITextSelection, or just ISelection, should also be considered instead of just returning true if it's not a IStructuredSelection.

I am looking into adding Tabbed Properties support to the JavaEditor and would need the ability to filter the sections based on a text selection in the editor.  bug 154781 is the related bug
Comment 1 Anthony Hunter CLA 2006-09-06 13:45:45 EDT
This is an enhancement request.

If I have a ISelection that is a ITextSelection, how in a generic way can I determine what type the ITextSelection is to match it to input types?
Comment 2 David Williams CLA 2006-09-06 16:44:45 EDT
A little knowledge is a dangerous thing :) [I'm not intimately familar with the use case] ... but, often, ITextSelections are used to look up "partition type" from the document, to know what other processor to invoke. Is this such a case? 

Comment 3 Karen Butzke CLA 2006-09-07 12:00:47 EDT
I am unsure about David's comment, so I will just respond to Anthony's for now.

I don't know of a way to generically take an ITextSelection and turn it into an
input type. I would like to see the API expect an instance of
IStructuredSelection instead of instanceof checks throughout the tabbed
properties framework.  We could add to the extension point an interface with a
method that takes an ISelection and returns an IStructuredSelection.  This
would give the ability in one location to take the ITextSelection, find my
model object that exists at that location in the java source code, and wrap it
in a StructuredSelection for the Tabbed Properties framework.  The default
implementation of this method would just cast the ISelection to an
IStructuredSelection, or do an instanceof and wrap it if it's not already an
IStructuredSelection.  Do you think this would break existing users?  It seems
to me that current implementations are all using IStructuredSelection anyway.

What do you think of this?
Comment 4 Anthony Hunter CLA 2006-09-07 15:01:39 EDT
I do not understand how you need to fix TabbedPropertyRegistryClassSectionFilter.appliesToSelection().

Can you create a patch with a proposed solution that works for you?
Comment 5 Karen Butzke CLA 2006-09-07 16:04:44 EDT
Created attachment 49659 [details]
new extension point attribute for providing an IStructuredSelection

TabbedPropertiesPatch.txt is a suggested solution to support ISelections other than IStructuredSelection in the tabbed properties framework.  I'm not suggesting this as a release-ready patch, it is just to help explain myself.  

You'll see that I added a new attribute (structuredSelectionProvider) to the propertyContributor extension point. In my implementation of the new interface IStructuredSelectionProvider(not included in the patch) I take the given ISelection, determine my own model object from it, and wrap it in a StructuredSelection and return that back to the framework to continue on its way.  

This solution allows me to only convert from the ITextSelection to my model object in one location.  Otherwise I would have to do this in the TypeMapper, the LabelProvider, as well as in each Section.  The tabbed properties article on eclipse.org gives the example using TreeNodes and the ITypeMapper to return the appropriate input class. In that example you would also have to pull the model object out of the TreeNode in the LabelProvider and each Section.  You could instead do this in one location, the IStructuredSelectionProvider. I don't believe the TypeMapper would even be necessary anymore for this example.
Comment 6 Karen Butzke CLA 2006-10-17 10:22:37 EDT
Created attachment 52121 [details]
adds ISelectionConverter as an option for property contributors

A new patch based on feedback from bug 154781.  Just a rename of the interface and some comments added to the class
Comment 7 Anthony Hunter CLA 2006-11-09 09:56:06 EST
The proposal is currently not acceptable. It makes changes to public API. 

I am not really sure why some of the changes are needed:

-	public boolean appliesTo(IWorkbenchPart part, ISelection selection);
+	public boolean appliesTo(IWorkbenchPart part, IStructuredSelection selection);

IStructuredSelection extends ISelection already.
Comment 8 Karen Butzke CLA 2006-11-13 11:51:54 EST
Created attachment 53752 [details]
New patch without current api changes

The same patch without the changes to the existing api.  I thought it made sense to change this since the implementation requires IStructuredSelections anyway, but since this would change public api, it won't work.
Comment 9 Anthony Hunter CLA 2008-06-05 23:29:07 EDT
I reviewed this patch again and I am not convinced we need to change the tabbed properties framework to accomplish what you want to do.

The core of the request is:

 	public void selectionChanged(IWorkbenchPart part, ISelection selection) {
-		setInput(part, selection);
+		ISelection convertedSelection = selection;
+		ISelectionConverter selectionConverter = registry.getSelectionConverter();
+		if (selectionConverter != null ) {
+			convertedSelection = selectionConverter.structuredSelection(selection);
+		}
+		setInput(part, convertedSelection);
 	}

Given that selectionChanged() is public, there is no reason why you cannot extend TabbedPropertySheetPage in your application layer. Right?
Comment 10 Karen Butzke CLA 2008-06-20 11:32:33 EDT
You are right, I can solve the problem by extending the TabbedPropertySheetPage and overriding the method selectionChanged(IWorkbenchPart, ISelection)

Since the goal here was to get the JavaEditor to use the TabbedProperties framework I thought it might be more appropriate for the framework itself to handle ITextSelection, especially since the API uses ISelection, but then expects IStructuredSelections in the implementation.  So, not a big deal to just extend the TabbedPropertySheetPage, but others who want to implement tabbed properties for text editors will have to do the same.
Comment 11 Anthony Hunter CLA 2008-06-20 11:55:47 EDT
(In reply to comment #10)
> You are right, I can solve the problem by extending the TabbedPropertySheetPage
> and overriding the method selectionChanged(IWorkbenchPart, ISelection)
> 
> Since the goal here was to get the JavaEditor to use the TabbedProperties
> framework I thought it might be more appropriate for the framework itself to
> handle ITextSelection, especially since the API uses ISelection, but then
> expects IStructuredSelections in the implementation.  So, not a big deal to
> just extend the TabbedPropertySheetPage, but others who want to implement
> tabbed properties for text editors will have to do the same.
> 

The framework does handle ITextSelection. The tests plugin for the framework demonstrates one way (selecting text creates tabs and sections based on the ITextSelection). IBM clients have text editors that use the framework based on ITextSelection as well.

(In reply to comment #1)
> If I have a ISelection that is a ITextSelection, how in a generic way can I
> determine what type the ITextSelection is to match it to input types?

There is no generic way, so you need to override and provide a custom implementation.