Feature/control append #48
Conversation
|
@GoldenZephyr also tagging you for review to get your take on the sequence merge semantics |
| @@ -54,6 +55,9 @@ target_compile_options(${PROJECT_NAME} PRIVATE -Wall) | |||
| set_property(TARGET ${PROJECT_NAME} PROPERTY POSITION_INDEPENDENT_CODE 1) | |||
| add_library(config_utilities::${PROJECT_NAME} ALIAS ${PROJECT_NAME}) | |||
|
|
|||
| add_executable(composite-configs app/composite_configs.cpp) | |||
There was a problem hiding this comment.
Haha I remember having the discussion whether we want a tool like that and we opted against it at the time, but seems reasonable to have in config_utilities 🙂
| @@ -42,12 +42,21 @@ | |||
|
|
|||
| namespace config::internal { | |||
|
|
|||
| enum class MergeMode { | |||
| //! @brief Combine the two trees, recursing into matching sequence entries | |||
There was a problem hiding this comment.
It might be good to also specify how conflicts are resolved here, I assume right will overwrite entries in left for entries with matching keys?
There was a problem hiding this comment.
Yeah, I can document the behavior better, I just forget if I changed anything with how keys are handled for maps or not (so I might stick it somewhere else)
config_utilities/include/config_utilities/internal/yaml_utils.h
Outdated
Show resolved
Hide resolved
| } | ||
|
|
||
| } // namespace | ||
| inline void mergeYamlMaps(YAML::Node& a, const YAML::Node& b, MergeMode mode) { |
There was a problem hiding this comment.
Minor: I don't have the header for this present right now but might be good to specify in the docstring that the MergeMode will be superseded by tags in the config (Which makes sense, just might be good to be clear about that). Maybe can call it default_mode or so?
There was a problem hiding this comment.
This and the sequence version actually aren't declared in the header (they're meant to be internal helper functions, so the default mode comes from the publicly accessible mergeYamlNodes). But will document more of the behavior for the public function
| } | ||
|
|
||
| inline void mergeYamlSequences(YAML::Node& a, const YAML::Node& b, MergeMode mode) { | ||
| const auto tag_mode = modeFromTag(b); |
There was a problem hiding this comment.
Minor: same as before, maybe default_mode?
Co-authored-by: Lukas Schmid <[email protected]>
Co-authored-by: Lukas Schmid <[email protected]>
|
Would it be possible to get a small, minimum viable example for how I'm supposed to run the commandline compositor (with e.g., two yaml files and simple tag interpolation)? |
At some point yes, though this is what I was going to use in the meantime until the interpolation / substitution parsing is actually stable and we've tested the compositing more |
@Schmluk Ended up coming across cases where I needed to (i) have fine-grained control over sequence update semantics (i.e., have some sequences that append when merging, and have some sequence that override) and (ii) start on interpolating values. Aaron and I have been talking about a better solution for the interpolation than tags (right now we can only override a key or value wholesale) but it's too involved to implement at the moment. This mostly has to do with the command line interface, but intention is that this is useful beyond that
Summary of changes:
!update, !append, !replace)!env VAR