brbzull0 commented on code in PR #13609:
URL: https://github.com/apache/trafficserver/pull/13609#discussion_r3913161541
##########
include/tsutil/YamlCfg.h:
##########
@@ -39,6 +39,26 @@ namespace Yaml
constexpr std::string_view YAML_BOOL_TAG_URI{"tag:yaml.org,2002:bool"};
constexpr std::string_view YAML_NULL_TAG_URI{"tag:yaml.org,2002:null"};
+ // Put an emitter into JSON output mode.
+ //
+ // yaml-cpp has no JSON output mode. The nearest equivalent is flow style
with every scalar double quoted. For the
+ // node shapes the callers here emit -- maps, sequences, scalars and nulls,
carrying no tags, anchors or aliases --
+ // that is JSON-compatible except for null: yaml-cpp writes `~`, which JSON
parsers reject. LowerNull writes the
+ // literal `null` instead. YAML resolves `~` and `null` to the same value,
so the output still reads as YAML.
+ //
+ // This is not a general YAML to JSON converter. A node that carries a tag,
an anchor or an alias still emits YAML
+ // syntax that JSON does not accept.
+ //
+ // Every emitter whose output reaches a JSON consumer must go through here.
Setting only some of the manipulators
+ // gives output that looks like JSON and parses correctly until some node is
null.
+ //
+ inline void
+ configure_json_emitter(YAML::Emitter &emitter)
+ {
+ emitter.SetNullFormat(YAML::LowerNull);
+ emitter << YAML::DoubleQuoted << YAML::Flow;
+ }
+
Review Comment:
`YAML::Emitter` can't be returned from a factory.
`lib/yamlcpp/include/yaml-cpp/emitter.h:43` deletes the copy constructor and
declares no move constructor, so the type is neither copyable nor movable. The
only form that compiles is `return YAML::Emitter{stream};` as a prvalue under
guaranteed elision — and that leaves nowhere to apply
`SetNullFormat(LowerNull)`, `DoubleQuoted` and `Flow` before the return, so the
call site has to configure it anyway. No invariant is gained.
A wrapper type that owns an `Emitter` and configures it in its constructor
would work, but it isn't the right trade here. JSON is not the dominant output
mode for this type: of the `YAML::Emitter` sites in the tree, six emit JSON and
now all six call `configure_json_emitter`. The rest emit YAML on purpose —
`src/config/ssl_multicert.cc:348` builds a YAML document immediately above the
JSON one at `:363`, and `src/config/storage.cc:762` and `:831` are the same
pair. A wrapper that made JSON the default construction path would have to be
bypassed at most call sites, which trades one thing to remember for another.
The helper plus the note at `include/tsutil/YamlCfg.h:52` documents the
invariant at the point where it matters. Leaving it as is.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
