brbzull0 commented on code in PR #13609:
URL: https://github.com/apache/trafficserver/pull/13609#discussion_r3905907654
##########
src/traffic_ctl/CtrlPrinters.cc:
##########
@@ -102,7 +102,7 @@ void
BasePrinter::write_output_json(YAML::Node const &node) const
{
YAML::Emitter out;
- out << YAML::DoubleQuoted << YAML::Flow;
+ ts::Yaml::configure_json_emitter(out);
Review Comment:
Fixed in f6ee3fb254 — `#include "tsutil/YamlCfg.h"` added directly to
`CtrlPrinters.cc`. You were right that it was only compiling through
`jsonrpc/ctrl_yaml_codecs.h` -> `shared/rpc/yaml_codecs.h`.
##########
include/tsutil/YamlCfg.h:
##########
@@ -39,6 +39,22 @@ 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 mode. The nearest equivalent is flow style with
every scalar double quoted, which is valid
+ // JSON for every node type except null: yaml-cpp writes ~, and no JSON
parser accepts that. LowerNull writes the
+ // literal null. YAML reads ~ and null as the same value, so output stays
readable as YAML either way.
+ //
+ // Every emitter whose output reaches a JSON consumer must go through here.
Setting two of the three manipulators
+ // gives output that looks like JSON and parses correctly until some node is
null.
Review Comment:
Correct, and reworded in f6ee3fb254. The claim is now scoped to the node
shapes these callers build, with the limitation stated explicitly:
> 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.
Confirmed against `EmitFromEvents::OnScalar`/`OnSequenceStart`/`OnMapStart`,
which all call `EmitProps(tag, anchor)`.
##########
include/mgmt/rpc/jsonrpc/json/YAMLCodec.h:
##########
@@ -21,16 +21,18 @@
#pragma once
#include <yaml-cpp/yaml.h>
+
+#include "tsutil/YamlCfg.h"
#include "mgmt/rpc/jsonrpc/error/RPCError.h"
#include "mgmt/rpc/jsonrpc/Defs.h"
namespace rpc::json_codecs
{
///
/// @note The overall design is to make this classes @c yamlcpp_json_decoder
and @c yamlcpp_json_encoder plugables into the Json Rpc
-/// encode/decode logic. yamlcpp does not give us all the behavior we need,
such as the way it handles the null values. Json needs
-/// to use literal null and yamlcpp uses ~. If this becomes a problem, then we
may need to change the codec implementation, we just
-/// follow the api and it should work with minimum changes.
+/// encode/decode logic. yamlcpp defaults to emitting null as ~, which is
valid yaml but not valid json, so every emitter here sets
+/// @c YAML::LowerNull to spell it literal null. Both spellings resolve back
to null when parsed as yaml, so accepting yaml input
+/// is unaffected.
Review Comment:
Done in f6ee3fb254 — the note now uses `yaml-cpp`, `YAML` and `JSON`, and
`~` is marked up with `@c`. It also names the new helper and drops two grammar
errors that predated this PR ("this classes", "plugables").
##########
include/mgmt/rpc/jsonrpc/json/YAMLCodec.h:
##########
@@ -28,9 +28,9 @@ namespace rpc::json_codecs
{
///
/// @note The overall design is to make this classes @c yamlcpp_json_decoder
and @c yamlcpp_json_encoder plugables into the Json Rpc
-/// encode/decode logic. yamlcpp does not give us all the behavior we need,
such as the way it handles the null values. Json needs
-/// to use literal null and yamlcpp uses ~. If this becomes a problem, then we
may need to change the codec implementation, we just
-/// follow the api and it should work with minimum changes.
+/// encode/decode logic. yamlcpp defaults to emitting null as ~, which is
valid yaml but not valid json, so every emitter here sets
+/// @c YAML::LowerNull to spell it literal null. Both spellings resolve back
to null when parsed as yaml, so accepting yaml input
+/// is unaffected.
Review Comment:
Addressed in f6ee3fb254 — canonical `yaml-cpp` / `YAML` / `JSON` spellings,
and `~` marked up with `@c`.
--
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]
