-
-
Notifications
You must be signed in to change notification settings - Fork 4.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
enh(TextProcessing): Allow providers and task types to declare a dynamic ID instead of using className #41088
Conversation
…mic ID instead of using className this allows AppAPI to register anonymous classes as TextProcessing providers and task types Signed-off-by: Marcel Klehr <mklehr@gmx.net>
(I'm sort of sad about the loss of beauty with this, but alas) |
Signed-off-by: Marcel Klehr <mklehr@gmx.net>
@marcelklehr What did we decide to do with settings (I mean adjust it to check for new interface with ID), is there any concerns to not do it the same as for Speech-to-Text? |
It's not as straightforward as with the STT API, because we need to be able to instantiate task types from their id. When we are using the class name as ID it works fine, but if we're using a getId() method we need to register task types as well. I'm not sure how we can do this in a backward-compatible way. |
Signed-off-by: Marcel Klehr <mklehr@gmx.net>
Decided with @julien-nc not to have task types implementable by ExApps. Then we can keep the class names for TaskTypes and this whole thing gets a lot easier. |
Signed-off-by: Marcel Klehr <mklehr@gmx.net>
Signed-off-by: Marcel Klehr <mklehr@gmx.net>
Signed-off-by: Marcel Klehr <mklehr@gmx.net>
Please review |
$provider = current(array_values(array_filter($providers, function ($provider) use ($preferences, $task) { | ||
if ($provider instanceof IProviderWithId) { | ||
return $provider->getId() === $preferences[$task->getType()]; | ||
} | ||
$provider::class === $preferences[$task->getType()]; | ||
}))); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indentation is off
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Small indent issue, otherwise looks good 👍
Signed-off-by: Marcel Klehr <mklehr@gmx.net>
this allows AppAPI to register anonymous classes as TextProcessing providers and task types
Checklist