Rewrite chart and set-attributes operators

[eliaskosunen]

Closes https://github.com/tenzir/issues/issues/1153

This PR brings the chart and set-attributes up to the spec described in the issue. Due to differences in the design, the common denominator between the implementations of these two operators is now VASTly reduced. Thus, set_attributes_operator_helper is removed.

• The operator syntax is changed from key=value to --key value/--key=value
• Due to above, argument_parser is used for parsing the command line (in chart). This leads to (arguably) improved error messages (see also step_03.ref in the diff), but also means that there's no longer any common parsing logic between chart and set-attributes:

# Before
error: could not parse `chart` operator
 --> <input>:1:20
  |
1 | from - read json | chart donut value=b x=e
  |                    ^^^^^^^^^^^^^^^^^^^^^^^ 
  |
  = note: !! syntax_error: chart type `donut` disallows field `x`

# After
error: unknown option `-x`
 --> <input>:1:42
  |
1 | from - read json | chart donut --value b -x e
  |                                          ^^ 
  |
  = usage: chart donut [--name <field>] [--value <field>] [--title <title>]
  = docs: https://docs.tenzir/operators/chart

• Attributes not set explicitly with the chart operator now have default values. --x-axis/--name defaults to the first field of the schema, --y-axis/--value to the second, and --title to the schema name. Due to this additional logic, set-attributes can't be used for the implementation of chart.
• All chart types are now defined in a single place, with easy-ish to understand syntax, in chart.cpp.
• chart is added to the documentation, and a changelog entry for it is added. set-attributes is kept as undocumented.
• "chart_operator" is added as a new feature flag

[mavam]

I know it might be too early to test this, but I wanted to give it a spin. It looks like the frontend is not yet picking up the attributes.

[dominiklohmann]

I know it might be too early to test this, but I wanted to give it a spin. It looks like the frontend is not yet picking up the attributes.

I, too, was curious. You need this patch for it to work.

diff --git a/libtenzir/builtins/operators/chart.cpp b/libtenzir/builtins/operators/chart.cpp
index dac05635e9..0b05b45771 100644
--- a/libtenzir/builtins/operators/chart.cpp
+++ b/libtenzir/builtins/operators/chart.cpp
@@ -261,7 +261,7 @@ struct chart_definition {
       = build_argument_list<std::optional<std::string>>(parser, optional_flags);
     parser.parse(p);
     configuration result;
-    result.emplace_back("type", attribute_value{std::string{type}});
+    result.emplace_back("chart", attribute_value{std::string{type}});
     // Go through the arguments,
     // and populate `result` with the specified attributes
     for (auto&& [attr, arg] : required_arguments) {
@@ -322,23 +322,23 @@ chart_definition chart_definitions[] = {
   {.type = "line",
    .required_flags = {},
    .optional_flags
-   = {{.attr = "x-axis", .flag = "-x,--x-axis", .type = flag_type::field_name, .default_ = nth_field{0}},
-      {.attr = "y-axis", .flag = "-y,--y-axis", .type = flag_type::field_name, .default_ = nth_field{1}},
+   = {{.attr = "x", .flag = "-x,--x-axis", .type = flag_type::field_name, .default_ = nth_field{0}},
+      {.attr = "y", .flag = "-y,--y-axis", .type = flag_type::field_name, .default_ = nth_field{1}},
       {.attr = "title", .flag = "--title", .type = flag_type::attribute_value, .default_ = schema_name{}},}},
   // `bar` is equivalent to `line`
   {.type = "bar",
    .required_flags = {},
    .optional_flags
-   = {{.attr = "x-axis", .flag = "-x,--x-axis", .type = flag_type::field_name, .default_ = nth_field{0}},
-      {.attr = "y-axis", .flag = "-y,--y-axis", .type = flag_type::field_name, .default_ = nth_field{1}},
+   = {{.attr = "x", .flag = "-x,--x-axis", .type = flag_type::field_name, .default_ = nth_field{0}},
+      {.attr = "y", .flag = "-y,--y-axis", .type = flag_type::field_name, .default_ = nth_field{1}},
       {.attr = "title", .flag = "--title", .type = flag_type::attribute_value, .default_ = schema_name{}},}},
   // `donut` chart is equivalent to `line` and `bar`, except
   // --x-axis is called --name, and --y-axis is called --value
   {.type = "donut",
    .required_flags = {},
    .optional_flags
-   = {{.attr = "name", .flag = "--name", .type = flag_type::field_name, .default_ = nth_field{0}},
-      {.attr = "value", .flag = "--value", .type = flag_type::field_name, .default_ = nth_field{1}},
+   = {{.attr = "x", .flag = "--name", .type = flag_type::field_name, .default_ = nth_field{0}},
+      {.attr = "y", .flag = "--value", .type = flag_type::field_name, .default_ = nth_field{1}},
       {.attr = "title", .flag = "--title", .type = flag_type::attribute_value, .default_ = schema_name{}},}},
 };

Thanks @eliaskosunen for the argument DSL, that made it really easy to modify this. Really good engineering as it'll also allow the app folks to modify the arguments without knowing any C++ at all.

[eliaskosunen]

Seems to be working after the applying the patch above

[mavam]

Just realized the awesomeness of positional arguments as dimensions, nice!