Add enable disable #101
Add enable disable #101
Conversation
Codecov Report
@@ Coverage Diff @@
## main #101 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 23 23
Lines 1282 1347 +65
=========================================
+ Hits 1282 1347 +65
Continue to review full report at Codecov.
|
|
this is all set @nclack |
Co-authored-by: Nathan Clack <nclack@gmail.com>
Co-authored-by: Nathan Clack <nclack@gmail.com>
Co-authored-by: Nathan Clack <nclack@gmail.com>
for more information, see https://pre-commit.ci
|
|
||
| def disable(self, plugin_name: PluginName) -> None: | ||
| """Disable a plugin""" | ||
| self._disabled_plugins.add(plugin_name) |
There was a problem hiding this comment.
You may want to to do the same check that a plugin is not already disabled, even if remove_contributions is no-op for consistency whether self.events.enablement_changed is emited.
| def discover(self, paths: Sequence[str] = ()) -> None: | ||
| # Discovery, activation, enablement | ||
|
|
||
| def discover(self, paths: Sequence[str] = (), clear=False) -> None: |
There was a problem hiding this comment.
I believe technically the super class also allows Path in addition to strings.
There was a problem hiding this comment.
good point. I think we want to remove discover from PluginManifest altogether and move that logic here. so might do in a followup pr
| self._manifests.clear() | ||
| if clear: | ||
| self._contrib = _ContributionsIndex() | ||
| self._manifests.clear() |
There was a problem hiding this comment.
it's not evented (I think ?) so you can also just assign an empty dict ? Would it be faster ? Unless something else keep reference to _manifest ?
There was a problem hiding this comment.
i'm trying to keep all private access within classes, even within npe2, so it should be fine just to set to empty dict
| for contribs in self._readers.values(): | ||
| for i, ctrb in reversed(list(enumerate(contribs))): | ||
| if ctrb.plugin_name == key: | ||
| contribs.pop(i) |
There was a problem hiding this comment.
Aren't we starting to make things really complicated using dicts, instead of using something flatter, and filtering in the relevant methods ?
There was a problem hiding this comment.
perhaps! not married at all to this one if you want to propose a better structure
There was a problem hiding this comment.
Ok, let' try to finish that and I'll suggest a refactor.
Co-authored-by: Matthias Bussonnier <bussonniermatthias@gmail.com>
This adds:
PluginManager.enablePluginManager.disableand new signals:
PluginManager.events.plugins_registeredPluginManager.events.activation_changedPluginManager.events.enablement_changed_ContributionsIndexhas been minimized, but not completely removed, and gains aremove_contributionsmethodALSO: note that this removes
discover()from the init method. So it must be called manuallysee also napari/napari#4086
I'm also taking the opportunity to re-order the methods and add docstrings on PluginManager (so the number of lines in this PR looks higher than it really is). They're starting to get scattered about without much logic