a couple of minor? adjustments #7
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
tweaks to support input stream iterator usage I found that the parser looks ahead one too many characters for my application. Consider these as suggestions as I don't fully understand either of these two modules
tweaks to support input stream iterator usage I found that the parser looks ahead one too many characters for my application. Consider these as suggestions as I don't fully understand either of these two modules And I'm not too hot on git either
|
Thank you Robert! We are taking a look at your changes. |
| cmake_minimum_required( VERSION 2.8 ) | ||
|
|
||
| project( json_spirit ) | ||
|
|
||
| if( CMAKE_COMPILER_IS_GNUCXX ) | ||
| message(STATUS "compiler is ${CMAKE_CXX_COMPILER_ID}" ) |
There was a problem hiding this comment.
Hope we can leave this one out
|
Thanks for helping out with the code, Robert. I have two meta-comments:
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I made a few changes in order to get this to build and work for my project on OSX with Clang
a) I had to tweak the CMake files to make things work with Xcode. I does build an Xcode project but I'm not sure that it's all correct. That is I don't think the Xcode tests work as expected. This is outside our scope - I think it's an issue regarding the CMake Xcode implementation. It does build the library however and that was enough for me. I seem to recall that I had some issues regarding the selection of static vs shared library build. It seems that the best setup is to build a static library which links to the shared library version of the standard libraries. This seems to contradict normal recommended practice of using all static or all shared libraries. Hopefully I made the right choice here - but I'll leave it to the experts. I'm hoping you don't think I'm the expert.
b) I need the parser to parse just part of the file. It turns out that it consumed one more character than necessary when used with an input stream iterator. I "fixed" it by doing an unmet after calling the parser. Again, I'm not sure is this really correct - I'll leave that to you.
Robert Ramey