add_pd_external

[umlaeute]

I would simplify the add_pd_external function by reducing the number of arguments from three to two: one for the binary output name and one for the source code. Currently, the function takes three arguments: the library name, the binary output name, and the source code.

There's an issue with using the ~ character in the add_library function. We have two options to solve this:

Not allows the ~ char: FATAL_ERROR if the user set a output with the ~ character, something like: "You can't use the ~ character, replace it with _tilde."

Automatic Replacement: Automatically replace the ~ character, but this approach introduces more problems (as seen in the the test implementation in my repo's dev branch).

Current: add_pd_external(library_name, binary_output_name, source_code)
Suggested: add_pd_external(binary_output_name, source_code)

What do you think?

Originally posted by @charlesneimog in #7 (comment)

[umlaeute]

• simplification is good
• your proposed change breaks compatibility (which is bad)
• having a ~ in the library name (aka "binary_output_name") is totally fine. it's only the setup function that cannot use it (and have to mangle it to _tilde)
• we do not want to have libraries that produce binaries like fiddle_tilde.dll
• i dont see your test implementation in the dev branch. you probably forgot to push it.

[umlaeute]

ah, i now see that the problem is not the "library_name", but the "Cmake target name" (or whatever the nomenclature). it's a CMake problem rather than a C problem.
(sorry for the confusion)

[umlaeute]

how about using named args (and a different name for the function, to keep compatibility):

pd_add_external(
   foo 
   SRC foo.c bar.c
)
pd_add_external(
  foo~
  TARGET foo_tilde
  SRC foo~.c
)

i think that

• named arguments are better suited for optional values than positional args
• we really should try hard to automatically pick a correct target name
• the original add_pd_external is more aligned with cmake naming conventions

[charlesneimog]

Sorry, I was a little busy.

My unique problem is that TARGET becomes a required argument when compiling objects with ~.

This seems hard to understand.

But at the same time, I can't think of a better option, hehe.

I will implement it.

[umlaeute]

My unique problem is that TARGET becomes a required argument when compiling objects with ~.

so why can't we mangle the name, in case it contains illegal characters?

[charlesneimog]

Mainly, if we change the target name, functions like target_link_libraries must also use the new name. For example, if we rename the target foo~ to foo_tilde, it would look like this:

pd_add_external(
   foo~
   SRC foo.c bar.c
)
target_link_libraries(foo_tilde PUBLIC fftw3)

Now, it might be best not to change the function and keep it with three arguments. I can't think of a good or best way to do this.