@marc.chevrier First up, sorry I didn’t get a chance to look at the original merge request for this feature when it was up for review. I should have caught this earlier, but am only just now getting to take a close look at this new command.
cmake_path() command being added for CMake 3.19 has subcommands of the form:
cmake_path(SUBCOMMAND <path> ...)
For all subcommands, the
<path> is required to be the name of a variable and the command will read that variable. I’m concerned that for at least some of the subcommands, this is unintuitive. Consider the
GET subcommand, which expects another option to specify the name of the output variable. For example:
cmake_path(GET <path> FILENAME <output>)
There doesn’t seem to be any reason why
<path> should be expected to be a variable name in this case. To me, it would be more intuitive if
<path> was a string, since this is how most other CMake commands work. If that were the case, I’d then write the docs for this command like so (here
<path> now is not the name of a variable, but an actual string):
cmake_path(GET <path> FILENAME <outputVariable>)
Other subcommands modify a variable in-place. For those where this makes sense, they could be documented differently to highlight this. If a name of a variable is expected, then in the docs we should use naming that makes this clear. For example:
cmake_path(APPEND varName [<input>...])
Note that the original documentation for the above looks like this:
cmake_path(APPEND <path> [<input>...] [OUTPUT_VARIABLE <output>])
It looks like this command can’t decide whether it wants to modify the variable in-place or expects you to provide a variable to store the result in. Other CMake commands do one or the other, but here we are trying to support both. I think this will be a source of confusion for users and we should offer one or the other. Since we have
list(APPEND) and friends as a precedent, one might consider following that example where it makes sense to modify a variable in place. Otherwise, require the name of the output variable to be explicitly provided and take a string rather than a variable name for the input.
I’ve just listed a couple of examples above, but I think we should revisit each of the subcommands and ensure we have behavior that is predictable and consistent with other CMake commands. I understand that these commands are likely based on the implementation of
std::filesystem, but that’s an internal implementation detail and we should first aim to be consistent with existing CMake behaviours.