Community
Participate
Working Groups
Many extension points define fields that require the user to specify a class name. Although fields that are named "class" do have a browse button beneath them, people have got used to the fact that content assists is availablealmost anywhere. I suggest that "class" fields should have content assist support.
Created attachment 41385 [details] Patch that adds CA to "class" fields This patch adds CA to class fields. There are two issues I didn't know how to cope with: a) Eclipse complains about discouraged accesses to the JDT UI libraries b) CA can only be invoked if you enter at least one character into the respective field. This very behaviour can be found in the "New Class Wizard" where I copied the CA code from.
Thanks Peter, but we would prefer an approach that does not make use of JDT internals. Mike to implement this feature. Brian to review Mike's patch.
Created attachment 43032 [details] Patch #1 This is an interim patch posted for back-up purposes only. I completedly removed all references to internal JDT classes and redeployed code as necessary into PDE with the exception of the following: import org.eclipse.jdt.internal.ui.JavaPlugin; import org.eclipse.jdt.internal.ui.viewsupport.ImageDescriptorRegistry; import org.eclipse.jdt.internal.ui.JavaPluginImages; import org.eclipse.jdt.internal.ui.viewsupport.JavaElementImageProvider; These classes are only used to access the class and interface images from within JDT for proposals. The necessary images will be copied from JDT into PDE when the functionality is completely finished TODO: * Use new CompletionProposal as a base class * Investigate alternatives and update references to deprecated content assist APIs -> org.eclipse.ui.contentassist.ContentAssistHandler; -> org.eclipse.jface.contentassist.SubjectControlContentAssistant; * Make proposals scope more configurable -> current JDT implementation proposes all classes on the classpath and no interfaces * Copy interface and class icon images into PDE from JDT where necessary * Improve code style and add comments * Investigate and further trim down unnecessary code deployed from JDT code
Created attachment 43624 [details] Patch #2 This is an interim patch posted for back-up purposes only. A thorough investigation of the alternative to the deprecated content assist APIs has led me to the very new and sufficiently different field assist APIs. I decided to try to create a bridge to the old APIs by using the new APIs with this new patch. Due to the newness of the field assist APIs, I discovered a variety of problems that need correcting before we can move off the deprecated APIs: * Field Assist Bug: Empty content proposals (whether IContentProposal[] is null or instantiated and empty) produces an empty proposal drop menu list when content assist is invoked -> expected no list to be generated and an audible sound to be emitted indicating no proposals available * Field Assist Bug: The content assist field length and alignment is improper on the form because the decorated field that utilizes the text field ignores or overrides the GridData specified * Field Assist Bug: The content assist field insists on instantiating / creating the text control when it itself is created causing integration of content assist fields to be messy and difficult -> the previous content assist API only required a reference to the underlying Text control in order to decorate / augment it with content assist functionality * Field Assist Bug: No way to associate and display images with proposals displayed in the proposal drop menu list using field assist APIs -> previous content assist API allowed this functionality * Restricting proposal scope to just Interfaces is extremely expensive performance-wise using IType.isInterface() method (and IType.isClass() method) -> bloats proposal generation time from under 1 second to 9 seconds -> assuming invokation of those methods are actually trying to read the underlying resources to cause such a degradation Moving forward, I will finalize the patch for the deprecated APIs and open the necessary bugs against the field assist APIs.
Attached Patch #3 ready for review. Attached Icon #1 needs to be copied into the following location: /org.eclipse.pde.ui/icons/elcl16/generate_interface.gif Patch Highlights * Added content assist functionality to the Activator field within the General Information section of the Overview tab on the plug-in manifest editor - Scope: non-JRE Classes currently on CLASSPATH * Added content assist functionality to the Class field within the Extension Element Details section of the Extensions tab on the plug-in manifest editor - Scope: non-JRE Classes and Interfaces currently on CLASSPATH * Fully commented * Completely eliminated all unnecessary code from JDT * Fixed search performance problems - Utilize JDT SearchEngine facility to compute initial proposals - Use SearchEngine's searchAllTypes() method over search() method to improve performance - Avoid use of expensive IType.isInterface() method * Implemented performance optimization - Filter initial proposal list based on additional user input rather than reperforming full JDT search * Implemented feature to widen initial proposal list if user inputs backspace Outstanding bugs affecting the use of field assist APIs https://bugs.eclipse.org/bugs/show_bug.cgi?id=145622 https://bugs.eclipse.org/bugs/show_bug.cgi?id=144522 https://bugs.eclipse.org/bugs/show_bug.cgi?id=144703 TODOs * Update deprecated content assist APIs to field assist APIs - org.eclipse.ui.contentassist.ContentAssistHandler; - org.eclipse.jface.contentassist.SubjectControlContentAssistant;
Created attachment 43991 [details] Patch #3
Created attachment 43992 [details] Icon #1
Overall, I like the patch. There are some minor things we can do to make this code a little more elegant. - In TypeCompletionSearchRequestor, lets try to use only 1 list to track the results from the last search (instead of fInitialResult and fCurrentResult). - Try to use a ListIterator when iterating through a list as in TypeCompletionSearchRequestor.filterCompletionProposals. (You will run into problems if you use the current method on 1 list since when you remove elements, your size() will change and you will exit the loop prematurely). - Try to take out the boolean fInitialSearch in TypeCompletionProcessor. See if we can listen for the "assist complete" event. - Since the Comparator is a small class, try rolling that up into an anonymous class. Some little things - Remember to externalize Strings (everyone forgets on their first couple patches). - Remove debug/performance information from comments. You can add them back later if you wish to check performance on your machine. Go ahead and post a patch with those changes and we should be in good shape.
With the new field assist support, do we even need the Browse... button anymore? If it goes away, we will gain more real estate for the text field.
Mike, Field assist seems to be available even when the editor is non-editable (i.e. when you open the editor on a blue plug-in from the Plug-ins view). This should not happen.
If this field level content assist were done using the platform DecoratedField or ContentAssistField class it would make implementing the UI portion of Bug 84420 easier (the translatable and error/deprecated indicator would become a decorator). Also, during 3.2, some of the platform dialogs moved to using a background colour to indicate mandatory fields. I will attach a screenshot that shows the extensions page with background colour on mandatory fields and a pencil beside translatable fields. It would be nice for the hover on the pencil to reveal the translated text.
Created attachment 44082 [details] Screenshot showing mandatory and translatable fields The background appears to eb too much on this screen, but the translatable decorator isn't too obtrusive (if the right hand sides all lined up)
The yellow is indeed too much, particularly that form editors are known to use white and system colours. I am not sure we need a background colour for the text field at all though. I find that usage of the * to be consistent with required fields in all forms on the web.
See attached Patch #4 and Icon #1 RE: Comment #8 Thanks Brian for the recommendations. Did the following: * Use only one list to track the initial search results - use another list to generate filtered search results and then discard it * Use a ListIterator when generating filtered search results * Use an assist completion event to discard initial search results and to track when to perform a full search versus a filtered search - eliminated use of boolean variable * Removed commented debugging code and externalized strings RE: Comment #9 I prefer to leave the browse button. Although I like the content assist feature I just coded ;), I think most users are accustomed to using the browse button and expect it to be there. RE: Comment #10 Did the following: * Disable content assist feature on fields if the editor is not editable
Created attachment 44151 [details] Patch #4
patch released. Targeted for 3.3
Brian, they just added new target milestones, so I'm retargetting to 3.3 M1 as it is more precise.
Thanks for implementing this feature!
you're welcome, Peter.
