Host Get Extension and Meson by mfisher31 · Pull Request #77 · free-audio/clap-helpers

mfisher31 · 2025-02-11T18:07:38Z

No description provided.

Trinitou · 2025-02-11T19:24:03Z

      virtual void requestCallback() noexcept = 0;

      virtual bool enableDraftExtensions() const noexcept { return false; }
+      virtual const void* getExtension (std::string_view extensionId) const noexcept { return nullptr; }


Suggested change

virtual const void* getExtension (std::string_view extensionId) const noexcept { return nullptr; }

virtual const void *extension(const char *id) const noexcept { return nullptr; }

rename for consistency with clap::helpers::Plugin::extension

just pass on the const char * -> let it up to the implementation to use std::string_view

format: void *extension( instead of void* extension ( seems more consistent with the rest to me

rename for consistency with clap::helpers::Plugin::extension

Plugin uses clapExtension and extension, but Host uses clapGetExtension so the corresponding method would be getExtension. However, the parameter type and name should be changed to const char *extension_id to match Host::clapGetExtension.

Should I change it back... probably should have looked here first. :)

Yeah, I actually think messmerd is more right than me :D
So maybe it would be ok for you to change the name back to getExtension...?

Trinitou · 2025-02-11T19:26:04Z

would be OK for me to add this. What do you think, @abique ?

My comment in discord was: if we add it we should add a ci test since it needs to work and most of us don’t use it ever.

I turned on ci here and it failed alas

Just for cmakr not even for meson

I've never used meson.
I'm not going to maintain it myself.
So as long as it is maintained by someone I don't mind.

I went ahead and made another branch ...

https://github.com/mfisher31/clap-helpers/tree/host-ext-access

... that has cherry-pick'd the extension access changes only.

My two cents is that a meson.build addition was a workaround anyway. If there was a cmake option, or something, to stop the download of clap without erroring then the cmake could be used directly as a cmake-subproject. (assuming the dev also handles the clap dependency on their own).

but the cmake already does that. The download is wrapped in if (NOT TARGET clap)

clap-helpers/CMakeLists.txt

Line 11 in 8558641

if (NOT TARGET clap)

Hmmm, my guess is that TARGET clap does not get registered when runs the checks and what not.

What does that mean?

when ci runs target clap is false so it downloads

you can see in the pr I pushed that the next main thing also gets fixed - if I merge that and you rebase your code should work. Just waiting for @abique to review

my comment was addressing yours about meson needing an if (which my pr also addresses)

I guess “what is the problem which requires a meson build file here and nowhere else”

Sorry I only really use cmake in passing. I just meant from my own perspective: If I used meson to subproject(...) in clap, then TARGET clap still returns false (not regisered in cmake). I'll do the rebase, kill the meson.build???, and also change back to getExtension() as @messmerd suggested?. Sorry this one went kind of haywire :)

baconpaul · 2025-02-14T14:06:32Z

I've started addressing the identified cmake problems #78

adds a control for MAIN/NEXT and PR against main pull main etc
Adds an option to not FATAL_ERROR if you don't have a clap target and dont have downloads on. Editorially wishes you luck in a warning.

abique · 2025-03-07T14:34:04Z

      }
-      return nullptr;
+
+      return self.getExtension(extension_id);


This would need to be called first and if that returns a valid ext pointer then return it.

hm, in clap::helpers::Plugin::clapExtension self.extension isn't called first. Should that be changed as well then?

Ahh yeah, didn't think about that. Doing so enables client code to override the default impl. I'll get it done.

Ok. I did a few things:

Squash all previous commits.

Rebase on next & force push

check overriden extension(...) and getExtension(...) before built-in implementations

remove the meson.build

remove #include <string_view> from host.hh

Turns out next works perfectly fine when using the cmake options: something like this:
https://github.com/kushview/element/blob/4d3d97114680a3312f1eddcf39b68e0ddcbdf845/meson/deps/meson.build#L5

# ... cmake = import ('cmake') clap_proj = cmake.subproject ('clap') clap_helpers_opts = cmake.subproject_options() clap_helpers_opts.add_cmake_defines ({ 'CLAP_HELPERS_DOWNLOAD_DEPENDENCIES': false, 'CLAP_HELPERS_NO_CLAP_IS_FATAL': false }) clap_helpers_proj = cmake.subproject ('clap-helpers', options: clap_helpers_opts) clap_dep = [ clap_proj.dependency('clap'), clap_helpers_proj.dependency ('clap-helpers') ] # ...

mfisher31 · 2025-03-07T14:42:45Z

Sorry for the delay on this. I just pushed the change to revert to getExtension(). I rebased on next, and am still in need of my meson.build. The situation for me is the same as before: if I use clap helpers from meson as a cmake subproject it either 1) blows up when downloading clap, or 2) can't find clap even though it too is part of the build as a cmake subproject.

All of the above is being worked out here:
https://github.com/kushview/element/blob/7eb89631aaa3ac43295239932ce7145e7c44923e/meson/deps/meson.build#L5

* Ignore generated files when used as a meson subproject

abique · 2025-03-25T07:49:35Z

Maybe split the assignment from the if and it'll compile :)
Appart from that, LGTM.

mfisher31 · 2025-03-25T10:26:12Z

Maybe split the assignment from the if and it'll compile :)

Whoops. probably should have checked one of the plugin (not host), projects I have on the side. 😎

abique · 2025-03-25T11:05:26Z

Thanks 👍

baconpaul · 2025-03-25T14:31:29Z

@@ -163,7 +165,7 @@ namespace clap { namespace helpers {
         // put draft ext here
      }



Am I reading this wrong or is it a pretty big change?

if I request host.get extension I no longer get the extension which routes to my internal virtual method for one of the mapped extensions?

It allows the host to shortcut the clap helper for a specific extension.
I think it makes sense.

Maybe I merged this too quickly, do you see a big issue with this change?

Maybe we could have two methods to make it clearer if that you're concern:

getExtension() which would be the fallback if the clap-helper didn't provide it

shortcutExtension() which would let the subclass provide the extension to use before the helper layer.

Would that make sense?

It allows the host to shortcut the clap helper for a specific extension. I think it makes sense.

but what if that extension is CLAP_EXT_GUI was my thinking. Then you get nullptr if you call get extension.

I definitely mix-and-match hostProxy.paramFoo and p = hostProxy.getExtension(PARAMS); p->foo(hostProxy->_host) in my code and expect both to kinda give the same function point.

The perhaps yeah 'underlyingExtension' or 'shortcutExtension' makes sense but I think this change will break some of my code.

This doesn't touch the PluginProxy or HostProxy.
It is only for the Plugin and Host.

Are you saying that you are using plugin-proxy to talk to your own plugin instance within itself?

ahhh did i misread?

i definitely use raw extensions from the host and the host proxy interchangeably.

let me check if i do the same with plugins. i think that's less likely.

* * Add host getExtension() handler * Ignore generated files when used as a meson subproject * host/plugin: check overriden extension access before built-ins * host.hh: remove string_view include; not used. * plugin: call extension on the 'self' object

Trinitou suggested changes Feb 11, 2025

View reviewed changes

mfisher31 changed the base branch from main to next March 7, 2025 14:26

mfisher31 force-pushed the meson-and-host-ext branch from 386ee7c to 15a450c Compare March 7, 2025 14:31

abique reviewed Mar 7, 2025

View reviewed changes

* Add host getExtension() handler

f8c8a51

* Ignore generated files when used as a meson subproject

mfisher31 force-pushed the meson-and-host-ext branch from a8b6a9d to f8c8a51 Compare March 25, 2025 02:00

host/plugin: check overriden extension access before built-ins

39dad0b

mfisher31 requested a review from Trinitou March 25, 2025 02:51

mfisher31 marked this pull request as ready for review March 25, 2025 02:51

host.hh: remove string_view include; not used.

9293c30

abique self-assigned this Mar 25, 2025

plugin: call extension on the 'self' object

f787220

mfisher31 requested a review from abique March 25, 2025 10:38

abique merged commit c75ab5f into free-audio:next Mar 25, 2025
14 checks passed

mfisher31 deleted the meson-and-host-ext branch March 25, 2025 11:14

baconpaul reviewed Mar 25, 2025

View reviewed changes

	virtual const void* getExtension (std::string_view extensionId) const noexcept { return nullptr; }
	virtual const void extension(const char id) const noexcept { return nullptr; }

		@@ -163,7 +165,7 @@ namespace clap { namespace helpers {
		// put draft ext here
		}

Conversation

mfisher31 commented Feb 11, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Trinitou Feb 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

baconpaul commented Feb 14, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mfisher31 Mar 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mfisher31 commented Mar 7, 2025

Uh oh!

abique commented Mar 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mfisher31 commented Mar 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

abique commented Mar 25, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Trinitou Feb 14, 2025 •

edited

Loading

mfisher31 Mar 25, 2025 •

edited

Loading

abique commented Mar 25, 2025 •

edited

Loading

mfisher31 commented Mar 25, 2025 •

edited

Loading