Skip to content
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

Plugin factory #74

Draft
wants to merge 5 commits into
base: next
Choose a base branch
from
Draft

Plugin factory #74

wants to merge 5 commits into from

Conversation

Trinitou
Copy link
Contributor

I tried to find a good solution for this common pattern and looked at various repositories how they do it.

It worked well for a local repo. But not tested on many platforms in many projects still (of course). So maybe you can try it out and let me hear what you think

Also some more questions

  • maybe the approach is too naive?
  • not completely sure about the misbehavior handling

@baconpaul
Copy link
Collaborator

I'm not sure what the goal is here? What is it you are trying to make easier to do?

@Trinitou
Copy link
Contributor Author

I'm not sure what the goal is here? What is it you are trying to make easier to do?

  • mainly just convenience so one can just include the header and has fewer lines of code to write (e.g. instead of always manually writing down the clap_plugin_factory construction again)
  • also it's good to have those checks in there whether the host does call everything correctly (at least during development)
  • in the end it also needs to be general enough so everybody can use it and it does not cause too much overhead (scanning should be as fast as possible of cause)

The current approach might not be optimal in every way concerning those goals. That's why draft and suggestions are very welcome.

  • e.g. the dynamic registerPlugin pattern might not be optimal for static single-Plugin claps (which probably is the other end of the spectrum) so maybe I should leave out the vector and leave the implementation more up to the user. So if it could be done in a universal way that better serves the static as well as the dynamic scenarios, it might be better. But on the other hand for the static cases it might be the easiest to manually fill out the clap_plugin_factory so despite of maybe the checks there is no big need for a helper
  • with dynamic I mean the "detect the available plugins dynamically depending on e.g. config files next to the clap file" patterns
  • one thing I do not cover yet is that if there is no plugin at all because of reasons the returned factory pointer should be zero

@baconpaul
Copy link
Collaborator

So most of my claps implement 3 or 4 factories right now. How would this pattern interact with that?

@Trinitou
Copy link
Contributor Author

So most of my claps implement 3 or 4 factories right now. How would this pattern interact with that?

Hm, this approach only covers one of the factories so extra helpers would be needed for the others. Or maybe it should be a clap::helpers::Entry class instead with factory methods to override by the user. (?)

  • so basically like clap::helpers::Plugin but with a std::unique_ptr<clap::helpers::Entry> that is made inside init and reset inside deinit
  • could work at least

@Trinitou Trinitou mentioned this pull request Jan 28, 2025
@abique abique force-pushed the next branch 3 times, most recently from 3a2625f to 6c4edba Compare February 18, 2025 15:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants