plugin: fix shared matchers and arguments in inherited plugins #6297
+83
−19
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Add metaclass for `Plugin` and set the `matchers` class attribute when a subclass is created, rather than checking if it's not `None` in the `pluginmatcher` decorator - Refactor `Matchers` and its base class, to allow passing `Matcher` objects to its constructor - Rename `Matchers.register()` to `Matchers.add()`, so it's consistent with `Arguments.add()` - Update tests
- Set the `arguments` class attribute when a subclass is created, rather than checking if it's not `None` in the `pluginargument` decorator - Update tests
bastimeyer
force-pushed
the
plugin/fix-shared-matchers-arguments
branch
from
dc31ead to
67d709a
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This fixes accidentally shared
matchersandargumentsreferences in plugin classes which inherit from existing plugin classes.It's not really a problematic issue, as it's very unlikely that the ancestor plugin class is used in a Streamlink session when it's already overridden by a plugin subclass in the same session, but it should still be fixed.
These changes unfortunately make the
PluginAPI docs look a bit weird.Fixing this also made me realize that we never deprecated the old
arguments = Arguments(...)definition style in plugin classes. The docs tell the user to always use the plugin decorators, but this is not enforced, hence why there's even a test case for the old plugin arguments definition. The matchers definition is theoretically also possible this way. This doesn't affect Streamlink's own plugins though, so it's probably not worth deprecating or directly removing this.Another thing that should probably also be changed and cleaned up in the future is how the
MatchersandArgumentsare defined.ArgumentandArgumentsare part of theoptionsmodule, because they are tied to theOptionsclass, while theMatcherandMatchersclasses are part of thepluginmodule (and thus thestreamlink.pluginpackage). Everything's actually only related to plugins, so theoptionsmodule shouldn't be in its current place in the rootstreamlinkpackage.streamlink.pluginalready re-exports everything from theoptionsmodule.