search

yhirose / cpp-peglib

A single file C++ header-only PEG (Parsing Expression Grammars) library
MIT License
879 stars 112 forks source link

Peglib as a CMake target #278

Closed glebzlat closed 1 year ago

glebzlat commented 1 year ago

It is not comfortable to use a library with FetchContent module, when the library is not specified as a target, so I added add_library(peglib INTERFACE). Also I added options to conditionally build tests, lint and examples, which are not building by default, modified .github/workflows/cmake.yml to build tests, and added a section about CMake into README.md.

yhirose commented 1 year ago

@edKotinsky thanks for the pull request. Could you take a look at #251 regarding BUILD_TESTS and #252 which shows my principle for CMake support in this project? Also I would like to keep the CMake support in this project as simple as possible, so that I can build everything quickly with mkdir build && cd $_ && cmake .. && make without memorizing any extra CMake options.

yhirose commented 1 year ago

So, I am ok to accept only the following lines, and I think this can benefit other users to install this header-only library easier. Thanks for your understanding!

add_library(Peglib INTERFACE)
target_include_directories(Peglib INTERFACE ${CMAKE_SOURCE_DIR})
glebzlat commented 1 year ago

@yhirose yes, of course. But when CMake fetches the project, it automatically executes its CMakeLists.txt file, so tests and examples will be built. Usually user expects when the dependency is fetched, it builds only itself.

yhirose commented 1 year ago

@glebzlat sorry for the late reply. I am not thinking to merge your changes. The only thing is about the change for BUILD_TESTS. Could you take a look at #251? Is there any strong reason to get back to PEGLIB_BUILD_TESTS?

yhirose commented 1 year ago

I implemented it in another way based on https://github.com/tinfoilboy/CTML/blob/master/CMakeLists.txt. Thanks for showing this idea!