scikit-build / scikit-build-core

A next generation Python CMake adaptor and Python API for plugins
https://scikit-build-core.readthedocs.io
Apache License 2.0
223 stars 47 forks source link

Allow toml lists in tool.scikit-build.cmake.define #874

Open alexreinking opened 3 weeks ago

alexreinking commented 3 weeks ago

The following entry leads to a cryptic error:

[tool.scikit-build.cmake.define]
SOME_LIST = ["A", "B", "C"]

However, I believe it should be allowed. Simply join the entries with a ; and issue a warning if any entry already contains a ; (which would split the "item" in CMake-land).

I think the tangible UX improvement comes when the list entries are many or long:

[tool.scikit-build.cmake.define]
MY_PROJECT_ENABLE_COMPONENTS = [
    "FancyComponent",
    "OptionalDependency",
    "ObtuseComponentWithAReallyLongNameWhyDidAnyoneThinkThisWasAGoodIdea",
    "Foo",
]
LecrisUT commented 3 weeks ago

Sounds good, but instead of erroring out if an inner ; is present, I think it should escape each layer as the list is constructed, so as to allow passing list of lists.

alexreinking commented 2 weeks ago

@LecrisUT - does that work? I thought lists of lists weren't really a thing in CMake.

LecrisUT commented 2 weeks ago

Yeah, last I've encountered it was in a PR on Catch2. As long as you have the appropriate escaping it works. Don't remember how the "" had to be used for those though.

alexreinking commented 2 weeks ago

So it does!

# test.cmake
cmake_minimum_required(VERSION 3.28)

set(compound "foo\\;bar" "baz" "quux\\;foo")

foreach (sublist IN LISTS compound)
   message(STATUS "!!!")
   foreach (item IN LISTS sublist)
     message(STATUS "${item}")
   endforeach ()
endforeach ()

Output:

$ cmake -P test.cmake
-- !!!
-- foo
-- bar
-- !!!
-- baz
-- !!!
-- quux
-- foo