Deprecate the include and exclude traits on PluginManager (#544)

This PR deprecates use of the `include` and `exclude` traits on the
`PluginManager` base class, on the grounds that those traits don't
currently work as intended (judging the intention from the tests), and
that it's not fully clear how they were intended to work.

The main clients for these traits seem to be the
`EggBasketPluginManager` and the `PackagePluginManager`, both of which
are now deprecated. The `EggPluginManager` also makes use of this,
though it duplicates and overwrites the functionality instead of
inheriting it from the `PluginManager` base class.

Technical details:

- the current `include` and `exclude` feed into private methods
`_is_included` and `_is_excluded`, which in turn feed into the
"protected" method `_include_plugin`, which is presumably intended to be
available for `PluginManager` subclasses to use. There are no other uses
of the `include` and `exclude` traits in the codebase.
- In the `PluginManager` itself, `_include_plugin` is used (a) when
iterating over the manager (via the `__iter__` method), and (b) in the
`get_plugin` method. However, the `start` method starts _all_ plugins
registered with the plugin manager, bypassing `_include_plugin`. I can't
tell whether this was intentional, but it seems unlikely, and the tests
suggest that it wasn't. I'd expect the result of
`iter(my_plugin_manager)` to match the actual set of plugins started and
stopped.
- In the `EggBasketPluginManager` and `PackagePluginManager`,
`_include_plugin` is also used to filter on entry point names (in the
first case) and on plugin ids (in the second).

There may well be a use-case for filtering plugins, but I think we
should design a working, self-consistent solution based on that use-case
when it comes up in practice. I'd also expect that we may want to allow
more general (and more pluggable) filtering than the current
`fnmatch`-restricted solution. In the meantime, I don't think there's
much value in keeping `include` and `exclude` around.

xref: #531

envisage/plugin_manager.py

@@ -12,6 +12,7 @@
 
 from fnmatch import fnmatch
 import logging
+import warnings
 
 from traits.api import Event, HasTraits, Instance, List, observe, provides, Str
 
@@ -82,6 +83,14 @@ def __init__(self, plugins=None, **traits):
         plugins use 'for plugin in plugin_manager'.
 
         """
+        if "include" in traits or "exclude" in traits:
+            warnings.warn(
+                "The 'include' and 'exclude' traits to PluginManager "
+                "are deprecated, and will be removed in a future version "
+                "of Envisage",
+                DeprecationWarning,
+                stacklevel=2,
+            )
 
         super().__init__(**traits)
 

envisage/tests/test_plugin_manager.py

@@ -143,14 +143,15 @@ def test_only_include_plugins_whose_ids_are_in_the_include_list(self):
         # plugins Ids.
         include = ["foo", "bar"]
 
-        plugin_manager = PluginManager(
-            include=include,
-            plugins=[
-                SimplePlugin(id="foo"),
-                SimplePlugin(id="bar"),
-                SimplePlugin(id="baz"),
-            ],
-        )
+        with self.assertWarns(DeprecationWarning):
+            plugin_manager = PluginManager(
+                include=include,
+                plugins=[
+                    SimplePlugin(id="foo"),
+                    SimplePlugin(id="bar"),
+                    SimplePlugin(id="baz"),
+                ],
+            )
 
         # The Ids of the plugins that we expect the plugin manager to find.
         expected = ["foo", "bar"]
@@ -167,14 +168,15 @@ def test_only_include_plugins_matching_a_wildcard_in_the_include_list(
         # plugins Ids.
         include = ["b*"]
 
-        plugin_manager = PluginManager(
-            include=include,
-            plugins=[
-                SimplePlugin(id="foo"),
-                SimplePlugin(id="bar"),
-                SimplePlugin(id="baz"),
-            ],
-        )
+        with self.assertWarns(DeprecationWarning):
+            plugin_manager = PluginManager(
+                include=include,
+                plugins=[
+                    SimplePlugin(id="foo"),
+                    SimplePlugin(id="bar"),
+                    SimplePlugin(id="baz"),
+                ],
+            )
 
         # The Ids of the plugins that we expect the plugin manager to find.
         expected = ["bar", "baz"]
@@ -189,14 +191,15 @@ def test_ignore_plugins_whose_ids_are_in_the_exclude_list(self):
         # plugins Ids.
         exclude = ["foo", "baz"]
 
-        plugin_manager = PluginManager(
-            exclude=exclude,
-            plugins=[
-                SimplePlugin(id="foo"),
-                SimplePlugin(id="bar"),
-                SimplePlugin(id="baz"),
-            ],
-        )
+        with self.assertWarns(DeprecationWarning):
+            plugin_manager = PluginManager(
+                exclude=exclude,
+                plugins=[
+                    SimplePlugin(id="foo"),
+                    SimplePlugin(id="bar"),
+                    SimplePlugin(id="baz"),
+                ],
+            )
 
         # The Ids of the plugins that we expect the plugin manager to find.
         expected = ["bar"]
@@ -211,14 +214,15 @@ def test_ignore_plugins_matching_a_wildcard_in_the_exclude_list(self):
         # plugins Ids.
         exclude = ["b*"]
 
-        plugin_manager = PluginManager(
-            exclude=exclude,
-            plugins=[
-                SimplePlugin(id="foo"),
-                SimplePlugin(id="bar"),
-                SimplePlugin(id="baz"),
-            ],
-        )
+        with self.assertWarns(DeprecationWarning):
+            plugin_manager = PluginManager(
+                exclude=exclude,
+                plugins=[
+                    SimplePlugin(id="foo"),
+                    SimplePlugin(id="bar"),
+                    SimplePlugin(id="baz"),
+                ],
+            )
 
         # The Ids of the plugins that we expect the plugin manager to find.
         expected = ["foo"]