Community
Participate
Working Groups
See bug 291755, comment 2 1) The filter ext. point should not use factories. I would use similar pattern to the builder extension, i.e. an abstract class (e.g. AbstractFileInfoMatcher) 2) The filter ext contributes matchers (not filters) that are used later by resource filters. This should be perhaps reflected in names. The name could be "resourceMatcher" or "resourceFilterMatcher"
Created attachment 150352 [details] Work in progress patch 20091023
Created attachment 150442 [details] Work in progress patch 2009.10.24
With this patch filters are created more like markers. You have to call IContainer#createFilter and pass an instance of FileInfoMatcherDescription. Right now FileInfoMatcherDescription is API, however I'm considering to have IWorkspace#createFileInfoMatcherDescription method that returns IFileInfoMatcherDescription. Still to do: - filters ext. point name should be changed - some changes in javadoc and implementation (e.g. the way filter id is generated - get rid of IContainer#removeFilter and add IResourceFilterDescription#delete (?) Serge, I think that you could try to adjust UI to changes in Filter API. If you have any comments on my changes, please let me know.
Created attachment 151067 [details] patch including the UI changes (minor change in core.resources) Includes the UI filter changes. The only change with the original patch is the method IFilterDescriptor.createFilter() is un-commented, since it is required by the compound filters in o.e.ui.ide (the AND, OR and NOT filters).
Created attachment 151077 [details] patch including the UI changes (minor change in core.resources) patch including the UI changes (minor change in core.resources) + additional check for null, and fix bugs preventing compound filters to be edited in the UI.
(In reply to comment #5) Serge, the patch seems to be corrupted. Apply patch tries to apply the UI plug-in changes to the core.resources plug-ins. Could you create it again and attach to the bug?
Created attachment 151276 [details] the same as before, hopefully not corrupted
Created attachment 151281 [details] The patch, again, but created on windows
Created attachment 151282 [details] The corrupted content
Created attachment 151286 [details] Filter patch, using the core.resources CompoundFileInfoMatcher
I released core.resources and core.tests.resources changes. Serge, could you handle the ui part?
Sorry I didn't see this earlier, but I'm having trouble understanding the new API. I thought the purpose of these changes was to simplify the API but it now looks more complicated. Here are some more specific comments: 1) There are now both "matchers" and "filters" which seem similar but slightly different. I might have been involved in discussing these terms before but I'm now confused about what the difference is between a matcher and a filter. There is both AbstractFileInfoMatcher.matches(IFileInfo) and IFileInfoFilter.matches(IFileInfo) that have the same API but are not related to each other. It would be nice to avoid introducing a new term for the same thing if we can avoid it. 2) There is now IResourceFilterDescription, IFileInfoMatcherDescription and IFilterDescriptor, which is confusing - I don't see why there are three different descriptions. The term "description" here doesn't seem to match other "description" objects in the resource API like IProjectDescription, which just encapsulates a set of state related to some other concrete object: IProjectDescription desc = workspace.newProjectDescription("foo"); desc.setComment(...); project.create(desc); ... desc.setComment(...); project.setDescription(desc); In this case there is no IResourceFilter to go with the IResourceFilterDescription. I think this interface should simply be called IResourceFilter like it was in e4 (drop the "description" from the name). 3) What happens if a client does this: IResourceFilterDescription[] descs = container.getFilter(); descs[0].setType(IResourceFilterDescription.INCLUDE_ONLY); Does that actually cause the filter to be modified? I think there shouldn't be any setter methods on here. The only way to change a filter should be to remove the old filter and add a new one. 4) Overall there are now a lot of new API types here just for filters: - AbstractFileInfoMatcher - CompoundFileInfoMatcher - FileInfoMatcherDescription - IFileInfoMatcherDescription - IFilterDescriptor - IResourceFilterDescription This is a lot of API, and confusing for clients I think to see so many types. Some of this is "SPI" which is intended for people contributing filter types, rather than simply using filters. These should maybe be in a separate package like we have for refresh participants (org.eclipse.core.resources.refresh). That way most clients would just see a more simple API - hopefully just something like IResourceFilter representing an existing filter, and IFilterDescription that represents information about a filter that might not exist yet. 5) There are various missing @since tags. Please setup API baselines so these problems are caught before releasing.
(In reply to comment #12) > 1) There are now both "matchers" and "filters" which seem similar but slightly > different. I might have been involved in discussing these terms before but I'm > now confused about what the difference is between a matcher and a filter. > ... > It would be nice to avoid introducing a new term for the same > thing if we can avoid it. Yes. We have discussed it earlier. The conclusion was to use an abstract class instead of an interface. So IFileInfoFilter should be removed, since the filter ext. point consumes AbstractFileInfoMatcher now. We call it matcher since this is what this class actually does. It matches IFileInfo objects. You mention that IFileInfoFilter is a general class that can be used by others that's why I leave it there. However I'm voting to remove it since this class is not used right now. > 2) There is now IResourceFilterDescription, IFileInfoMatcherDescription and > IFilterDescriptor, which is confusing - I don't see why there are three > different descriptions. The term "description" here doesn't seem to match other > "description" objects in the resource API like IProjectDescription, which just > encapsulates a set of state related to some other concrete object: > IProjectDescription desc = workspace.newProjectDescription("foo"); > desc.setComment(...); > project.create(desc); > ... > desc.setComment(...); > project.setDescription(desc); > > In this case there is no IResourceFilter to go with the > IResourceFilterDescription. I think this interface should simply be called > IResourceFilter like it was in e4 (drop the "description" from the name). IResourceFilterDescription is just a description of a filter. It contains just the type (exclude, include, etc.) and the description of the matcher that should be used. I agree that the setters should be removed. > 4) Overall there are now a lot of new API types here just for filters: > > - AbstractFileInfoMatcher > - CompoundFileInfoMatcher This is SPI. > - FileInfoMatcherDescription > - IFileInfoMatcherDescription > - IFilterDescriptor > - IResourceFilterDescription I think that API should contain IFilterDescriptor (similar to IProjectNatureDescriptor), IResourceFilterDescription and IFileInfoMatcherDescription. I would remove FileInfoMatcherDescription and add a method (maybe to IWorkspace) to create IFileInfoMatcherDescription objects. Moreover IFileInfoMatcherDescription would just contain getters. Moreover we agree with Serge that 'filters' extension point should be renamed to something like 'filterMatchers' since it allows to contribute matchers not filters. > 5) There are various missing @since tags. Please setup API baselines so these > problems are caught before releasing. My p2Agent seemed to fail and my 3.5 profile didn't contain all bundles, so I couldn't see the problem. I'll fix it. I know that javadocs are not updated right now. I'm on it right now, so I'll just open a bug to show that it is not forgot.
Created attachment 152378 [details] The ext. point renamed to 'filterMatchers'
Marking FIXED.
