-
Notifications
You must be signed in to change notification settings - Fork 26
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Feature/generalize parser #80
Conversation
… Used in multio to directly parse JSON to Metadata without using eckit::Value inbetween
I'm getting a compile failure with this: /home/d03/frwd/cylc-run/eckit-80/share/mo-bundle/eckit/src/eckit/parser/ObjectParser.cc:21:1: error: declaration does not declare anything [-fpermissive] This is with gcc 7.3.0 |
ah yes. I hoped that the new CI will compile some stuff for me without further effort. |
I still would like to use the CI for testing compilation with different compilers. However on the workflow it says than an reviewer has to approve it - but I think the workflow can be triggered separately from approving the whole PR? |
This branch is based on the other PR with SFINAE capabilites for eckit::Translator - we may also just merge this one.
Changes in the branch require C++17 - before C++11 dependencies like metkit, fdb etc. have also compiled because compilers just emitted warnings. Hence PR for other repositories have been created as well.
Motivation:
ObjectBuilder
that is describing how to create an object. The traitObjectBuilderTraits
is checking for some requirements to describe the builder (will look nicer with concepts)..Changes:
eckit::Value
exchanged to specific calls toObjectBuilder
parseString
is returningstd::string
as parsing is all about strings and sometimes string values need to be accessed directly (not hidden in a generic type) to handle keys or multiline strings.Counted
(attach
/detach
calls were very confusing and opaque to me). Now one class with tags (enum class) and a nested union is used.Moreover a
std::deque
was used to load items - however as only 1 item was loaded ahead, astd::optional
is now used instead.YAML fixes:
With these changes, it is now also very simple to create custom value types (e.g. with
std::variant
) and replace configurations (if required at all).I tried to to do a clean and sharp cut, if there are more things you think we should change let me know. The basic parsing logic is still the same.