Community
Participate
Working Groups
There are lot of useful functionality in RegExContentProposalProvider and it would be really handy to be able to reuse this class in custom plugins to avoid copying.
I assume you meant this to go to Text.
See bug 87766 comment 1 for details.
I don't think it is fair comment. Also not sure why search feature would not support certain regexps that are supported by JRE.
From a user's point of view (and for consistency), it is desirable that when a product is built on top of Eclipse and regular expressions are used in multiple places, those regular expressions all work the same. Eclipse is a powerful platform and should foster such unifications and consistency across all extenders. I'd think that since the relevant Eclipse plugins are built on top of Java 1.4, the Java API Regex.compile() defines what is the canonical syntax and feature set of regular expressions to be used inside Eclipse. I'd not expect any component to use different kinds of regular expressions. Therefore, I do think that access to the RegExContentProposalProvider should be made public. Since there is a "standard" form of Regular expressions, I cannot understand the argument of clients potentially getting out of sync. The class should be pushed down into org.eclipse.jface.fieldassist just like SimpleContentProposalProvider, and either made public or a public factory method provided. In order to provide consistency, I would also like to see the summary of this bug report changed and NOT allow custom extensions to the RegExContentProposalProvider. Eugene, it's your bug -- what do you think?
Fair enough. Since it mangles with text it would be placed into jface.text. OK? No extension would be provided though.
*** Bug 87766 has been marked as a duplicate of this bug. ***
Looking at http://www.eclipse.org/eclipse/development/eclipse_project_plan_3_3.html I see that org.eclipse.jface has JRE prerequisite "Foundation 1.0" which I don't think includes the Regex class. Therefore I agree that org.eclipse.jface.text is the better home, it already has prerequisite "1.4" which includes the JRE Regex class. Thanks for picking this up!
(In reply to comment #4) > I'd think that since the relevant Eclipse plugins are built on top of Java > 1.4, the Java API Regex.compile() defines what is the canonical syntax and > feature set of regular expressions to be used inside Eclipse. I'd not > expect any component to use different kinds of regular expressions. I might be wrong, but I think that Eclipse text editors aren't using regexp engine from the Java API. Also Java regexps have some issues and can get into the endless loop on certain regexps that are using lookaheads. > In order to provide consistency, I would also like to see the summary of this > bug report changed and NOT allow custom extensions to the > RegExContentProposalProvider. Eugene, it's your bug -- what do you think? The reason I asked to make it non-final, so it could be used to support some other microlanguages. For instance provide completion for variables like ${varName} into the same text as regexps (like one used in the Mylar web repository connector). I am not sure if something like that can be supported by some kind of composite completion provider.
(In reply to comment #8) > I might be wrong, but I think that Eclipse text editors aren't using regexp > engine from the Java API. I don't think the actual engine used matters here -- what matters is the Regex API being used, in this case a slightly modified variant of Perl Regexps as far as I know. And I'd think that this Regex API could (and should) be common across all user-visible widgets in an Eclipse based appliation that support Regexps. > I am not sure if something like that can be supported by > some kind of composite completion provider. I guess that even if the RegExContentProposalProvider were final, or available only by a factory method returning IContentProposalProvider, you could always use a Decorator to add your specific completion proposals. But I didn't check for that very thoroughly.
We use java.util.regex. >I guess that even if the RegExContentProposalProvider were final, At the moment I see no reason why we should disallow subclassing.
Can we have a more reasonable target milestone for this? Since API change is involved, it looks like this should be done before M6. We now have another requirement for this, see bug 168000 comment 11. Is there anything we can do to help making the change really happen?
>Can we have a more reasonable target milestone for this? What's this? 3.4 is quite reasonable. Of course I'm happy to get a patch earlier.
Created attachment 83235 [details] Patch moving code to org.eclipse.jface.text.contentassist Since this is an API change, it needs to be completed before 3.4M6. To get this started, attached patch just moves RegExContentProposalProvider (and associated RegExMessages.java, RegExMessages.properties) into package org.eclipse.jface.text.contentassist and makes RegExContentProposalProvider public / non-final as previously agreed. Some open questions that occurred to me: * The constructor RegExContentProposalProvider(boolean isFind) looks a little bit odd when looking at the new API from a point of view not involving a find/replace dialog. Could this be improved, at least improving Javadoc in order to explain what the difference between isFind == true or false is? * I left the static nested class ProposalComputer private for now, should this be protected to allow easier extension (Eugene what do you think)? * As this is going to be new API, it looks like we'll need unit tests for it? I modified the @since tag from 3.2 to @since 3.4 since this would be the time that this class first appears in API (users would not be interested in its history being non-API).
>* As this is going to be new API, it looks like we'll need unit tests for it? Wouldn't hurt ;-) We also have to move the doc/F1 help. Marking for M5 but might be able to look at it for M4.
Hm, the much I'd like to help, here's where my knowledge ends :-) I Cannot find any context help ID's related to the RegexContentProposalProvider, they seem to be linked directly to the find/replace dialog. What I did notice was the component.xml file in org.eclipse.jface.text, I'm not sure if that needs to be updated to reflect the new API? The existing entries seemed odd because there were so few, and "instantiate=false" for ContentAssistEvent seems to contradict what it's Javadoc says... <package name="org.eclipse.jface.text.contentassist"> <type name="ContentAssistEvent" instantiate="false" subclass="false"/> <type name="RegExContentProposalProvider" instantiate="true" subclass="true"/> </package>
>Hm, the much I'd like to help, here's where my knowledge ends :-) That's OK. I'll do that. The component.xml needs to be updated in case of restrictions that aren't visible from the code, e.g. if there's a non-final type with Javadoc that says 'not to be subclassed by clients'.
Added FindReplaceDocumentAdapterContentProposalProvider that allows subclassing. Sorry I ignored the patch as I had to make various changes (e.g. NLS must not be used inside JFace Text. Please provide feedback asap as M6 is the API freeze. Available in builds > I20080318-0800.
