Community
Participate
Working Groups
Hi, I am a committer for the PDT project. As a Java developer, I always missed the automatic content assist triggered by any character, and not only by the '.' or any other configurable list of characters. for example, if the user types the following: class MyClass extends | (the "|" is the carret position) in other words if the user put a space after the word "extend" a list of classes to override will automatically pop up. Lately, we received several similar requests from the PDT users about the content assist. This can only emphasise the need for this feature. I saw that in bug #101420 someone asked for a similar feature, but I guess my request is simpler. I'm willing to implement it myself as a patch if you will approve it as a necessary feature (and assuming you are not working on something like this currently). Thanks, Guy
*** This bug has been marked as a duplicate of bug 159157 ***
Our suggestions are different, he asks to expend the triggers range, and I think we need to extend the mechanism and give the processors the option to be activated regardless to the user typing. I also think we should integrate the solution inside org.eclipse.jface.text.contentassist.ContentAssistant and not as an external plugin.
>Our suggestions are different I know it's a little bit different but it goes into the same direction and thought this might be enough for you. There are no plans to put a general trigger mechanism into content assist as this would affect typing performance considerably. Feel free to subclass content assistant and provide this feature in your editor. >I also think we should integrate the solution inside >org.eclipse.jface.text.contentassist.ContentAssistant >and not as an external plugin. Sure. That's why the other bug is still open.
The problem is that I can't access some important data members like fContentAssistSubjectControlAdapter for instance since they are private (and its Type ContentAssistSubjectControlAdapter is package). Any chance you could change some of those members and types in order to make it more available for subclassing. (I'll send the full list of members and types I need if you agree).
Access to fContentAssistSubjectControlAdapter could be granted but not to the class. fContentAssistSubjectControlAdapter could be be access via type 'IContentAssistSubjectControl'.
Here is the list of things I need access for: 1. running the following functions fProposalPopup->isActive() fProposalPopup->showProposals(boolean) fContextInfoPopup->isActive() fContextInfoPopup->showContextProposals(boolean) prepareToShowCompletions() promoteKeyListener() 2. option to set fLastAutoActivation 3. We want to override manageAutoActivation() for that we need access for: fContentAssistSubjectControlAdapter - we need access for the member according to IContentAssistSubjectControl interface fAutoAssistListener - only getter and setter will do. please reconsider
I assume you would write your own subclass of the ContentAssist then, right?
yes
I suggest you attach a patch (as small as possible) here that adds protected methods to ContantAssistant and fits your needs. I can then review.
Created attachment 71837 [details] patch allowing subclassing and replacing the AutoAssistListener What I did is added an option to replace the AutoAssistListener and question the ContentAssistant regarding fContextInfoPopup and fProposalPopup state. Of course we can extend the support for subclassing even more, I would be glad to debate we you about it if you like. Thanks.
The attachment is not a valid patch. You need to load the 'org.eclipse.jface.text' project from CVS, modify the file(s) and then create the patch.
Created attachment 71882 [details] patch allowing subclassing and replacing the AutoAssistListener hope it's ok now...
>Of course we can extend the support for subclassing even more, I would be glad >to debate we you about it if you like. I want to keep it to a minimum unless we have a clear use case. The last 6 years went quite well the way it's currently implemented ;-) Now to your patch: I can accept something along those lines. Here are the detail comments: - since there's already a public setAutoActivationDelay I would simply add a corresponding getter as public API. This makes the story consistent. - isProposalPopupActivated ==> isProposalPopupActive - the 'not' in isContextInfoPopupNotActivated is ugly. I would change the name to isContextInfoPopupActive and adjust the logic. - the new methods need Javadoc and you need to put your name into the header comment along these lines below the initial contributor: full name, e-mail - bugNr bugSummary
Created attachment 71995 [details] patch with fixes I changed the patch as you requested. One point though > - the 'not' in isContextInfoPopupNotActivated is ugly. I would change the name > to isContextInfoPopupActive and adjust the logic. the method functionality was: (fContextInfoPopup != null && !fContextInfoPopup.isActive()) (like in line 312 in the original file). changing the name to a "positive" one, will get us to: (fContextInfoPopup == null || fContextInfoPopup.isActive()) which means it will return that the fContextInfoPopup is active although the fContextInfoPopup is null. That's why I had to split this method into two, isContextInfoPopupInitiated and isContextInfoPopupActive.
I don't see why you would need isContextInfoPopupInitiated(). The field is not null as soon as content assist is installed. Please add full Javadoc as in all other methods of the class and don't forget to add the @since 3.4 tag.
> I don't see why you would need isContextInfoPopupInitiated(). The field is not > null as soon as content assist is installed. The same reason you do in line 312.
Created attachment 72023 [details] update documentation hope its OK now
Do you only need the new is* methods in your AutoAssistListener subclass?
and the getAutoActivationDelay(). As I said I tried to keep it as minimal as possible. But I guess if we wanted to make it even more accessible for subclassing than we could also open the CompletionProposalPopup and ContextInformationPopup for the public.
I don't like the two isContext* methods. I suggest we add protected getContextInfoPopup() and getContextInfoPopup() to the AutoAssistListener. This way you can do with them whatever you like.
But then we'll have to open ContextInformationPopup and CompletionProposalPopup for the public. is that ok with you?
I've thought about this again and there is really no need to have isContextInfoPopupInitiated(). I've applied your patch - isContextInfoPopupInitiated() + small modifications inside ContentAssistant to use the new is* methods + cosmetics (formatting, wording of Javadoc)
Thank you very much, it was pleasure working with you.
