feature: allow additional arguments after shorthand syntax

[SGSSGene]

This allows to combine the shorthand syntax with additional arguments:

CPMAddPackage("gh:nlohmann/[email protected]" OPTIONS "JSON_BUildTests OFF")

This is much shorter than the longer syntax way of writing:

CPMAddPackage(
  NAME nlohmann_json
  VERSION 3.9.1
  GITHUB_REPOSITORY nlohmann/json
  OPTIONS
    "JSON_BuildTests OFF"
)

[TheLartians]

Hey, thanks for the PR! In general I like the idea of mixing the short syntax with additional arguments and think this is a good approach in doing so. Could you perhaps add an integration test case for this?

Also I'm not sure but it could be that the cmake-format config might need to be updated for this new syntax.

[SGSSGene]

Hey, thanks for the PR! In general I like the idea of mixing the short syntax with additional arguments and think this is a good approach in doing so. Could you perhaps add an integration test case for this?

Also I'm not sure but it could be that the cmake-format config might need to be updated for this new syntax.

Yes, there are a few things that need to be updated. I wanted to discuss, if there is any chance on success before investing to much time.
I will look through everything and update this PR

[TheLartians]

Thanks, the changes look good, just want to make sure that the test actually does what it should. One thing that I notice from the readme changes is that having a mix of single + named arguments looks quite inconsistent and therefore harder to read.

How would you feel if we instead make a compromise by introducing a URI argument that takes the role of the single argument instead?

This would allow us to do

CPMAddPackage(
  URI "gh:nlohmann/[email protected]"
  OPTIONS "JSON_BuildTests OFF"
)

Instead of

CPMAddPackage("gh:nlohmann/[email protected]"
  OPTIONS "JSON_BuildTests OFF"
)

To me this looks much nicer, even though I know CMake itself likes to use this mix of syntax. Of course, using only the single argument syntax should continue to be valid.

[TheLartians]

I feel like having this run as part of a larger test makes reasoning about the test itself difficult. Do you think you could move checking the shorthand extra arguments into its own isolated test case?

[SGSSGene]

Sure 👍

[SGSSGene]

Thanks, the changes look good, just want to make sure that the test actually does what it should. One thing that I notice from the readme changes is that having a mix of single + named arguments looks quite inconsistent and therefore harder to read.

How would you feel if we instead make a compromise by introducing a URI argument that takes the role of the single argument instead?

This would allow us to do

CPMAddPackage(
  URI "gh:nlohmann/[email protected]"
  OPTIONS "JSON_BuildTests OFF"
)

Instead of

CPMAddPackage("gh:nlohmann/[email protected]"
  OPTIONS "JSON_BuildTests OFF"
)

To me this looks much nicer, even though I know CMake itself likes to use this mix of syntax. Of course, using only the single argument syntax should continue to be valid.

I am not so sure of the URI idea. My biased view:
Pro:

• It does look nice.

Con:

• Not easy extendible from shorthand syntax, my main idea was that I can just slap on further 'OPTIONS', with this I also need to add a URI
• Harder to document. Currently we have two cases: 1. normal usage, 2. shorthand syntax. With URI we kind of have to split the 'normal usage' into two cases. I don't see how this can be documented in any understandable way. (Assuming we don't delete the 2. shorthand syntax)
• Also I think it is harder to implement (not so sure about this)

I like to think about it similar to how target_link_libraries works. First I set a free given parameter, and than I add all the other with PRIVATE,PUBLIC or INTERFACE.

[Richardk2n]

Gentleman, how is this coming along?
Is there anything preventing this from getting merged other than the URI or not URI question?

[SGSSGene]

The question of URI or not URI is up to @TheLartians . Both is fine to me, @TheLartians needs to decide what he would like.

The only big stopper for the URI idea for me is, I think URI is a bad keyword, since it is not a URI?!
Any argument for saying it is a URI or an alternative keyword would be helpful.

[SGSSGene]

@TheLartians : I created a alternative version, with an explicit URI #617.
The version with URI also does not set "EXCLUDE_FROM_ALL;YES;SYSTEM;YES;" by default.