Cmake build for msvc #41
Conversation
|
|
||
| using namespace std; | ||
|
|
||
| #elif defined(_WIN32) |
There was a problem hiding this comment.
I am actually not entirely certain what this is doing since it seems to work just fine If I comment it out the #define or leave it in. Not sure if it is a MSC_VER issue so it is needed in older versions. What I know if both the `#define timegm and using toml::timegm are active it doesn't compile.
There was a problem hiding this comment.
IIRC, code around here is for cygwin environment.
| function(add_toml_test target) | ||
| add_executable(${target}_test ${target}_test.cc) | ||
| add_executable(${target}_test ${target}_test.cc ../include/toml/toml.h) |
There was a problem hiding this comment.
Adding .h seems weird to me. Here, adding toml.h as an input src. It will work, though...
There was a problem hiding this comment.
maybe you just want to add toml.h as a dependency, right?
Then what we need is target_include_directories (or include_directories).
https://stackoverflow.com/questions/13703647/how-to-properly-add-include-directories-with-cmake
There was a problem hiding this comment.
The issue is not about code dependencies, I think that was working correctly as it was. It was about actually seeing the project in visual studio. And the basic issue was toml.h wasn't showing up anywhere in the project hierarchy, so the project was kind of a pain to work with as the main file wasn't showing up anywhere. So this isn't adding much if anything to the chain, but it does make it lot easier to work with the project and target in Visual studio, and shouldn't affect it in other builds.
The other option is to make a dummy target with just toml.h in it to work with, but this is what I have seen done in some other header only libraries and have used in the past.
There was a problem hiding this comment.
Hmm, I haven't cared for *.vcproj at al... I'm not so sure what the best practice is when considering all platforms, but ok for now.
src/build.h
Outdated
| @@ -288,7 +288,7 @@ inline toml::Value build_string_01(void) | |||
| toml::Value root((toml::Table())); | |||
| toml::Value* top = &root; | |||
|
|
|||
| top->setChild("s1", "I'm a string. \"You can quote me\". Name\tJos\u00E9\nLocation\tSF."); | |||
| top->setChild("s1", u8"I'm a string. \"You can quote me\". Name\tJos\u00E9\nLocation\tSF."); | |||
There was a problem hiding this comment.
sure will do. I will update and rebase and collapse the commits
…ws based systems interacted badly, so this change makes sure they both can't turn on at the same time on MSVC update some of the cmake code and one test to build and pass on MSVC
phlptp
force-pushed
the
cmake_build_for_msvc
branch
from
c91a1b9
to
20dc09f
Compare
If merged this PR will resolve a conflict in parser_complex_test in MSVC
It also adds folders to the CMAKE build and a few other items to make it easier to work with on Visual Studio and other IDE builds.
Fix builder_test::build_parse_string_01 which was failing on Visual studio, something with UTF strings, basically made the string literal u8 instead of a regular string literal in the build function.