Skip to content

Commit

Permalink
Switch plugin msgpack protocol to named format (nushell#12580)
Browse files Browse the repository at this point in the history
# Description
In conflict with the documentation, the msgpack serializer for plugins
is actually using the compact format, which doesn't name struct fields,
and is instead dependent on their ordering, rendering them as tuples.
This is not a good idea for a robust protocol even if it makes the
serialization and deserialization faster.

I expect this to have some impact on performance, but I think the
robustness is probably worth it.

Deserialization always accepts either format, so this shouldn't cause
too many incompatibilities.

# User-Facing Changes
This does technically change the protocol, but it makes it reflect the
documentation. It shouldn't break deserialization, so plugins shouldn't
necessarily need a recompile.

Performance is likely worse and I should benchmark the difference.

# Tests + Formatting
- 🟢 `toolkit fmt`
- 🟢 `toolkit clippy`
- 🟢 `toolkit test`
- 🟢 `toolkit test stdlib`
  • Loading branch information
devyn authored Apr 19, 2024
1 parent 9fb59a6 commit f2169c8
Showing 1 changed file with 2 additions and 2 deletions.
4 changes: 2 additions & 2 deletions crates/nu-plugin/src/serializers/msgpack.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ impl Encoder<PluginInput> for MsgPackSerializer {
plugin_input: &PluginInput,
writer: &mut impl std::io::Write,
) -> Result<(), nu_protocol::ShellError> {
rmp_serde::encode::write(writer, plugin_input).map_err(rmp_encode_err)
rmp_serde::encode::write_named(writer, plugin_input).map_err(rmp_encode_err)
}

fn decode(
Expand All @@ -46,7 +46,7 @@ impl Encoder<PluginOutput> for MsgPackSerializer {
plugin_output: &PluginOutput,
writer: &mut impl std::io::Write,
) -> Result<(), ShellError> {
rmp_serde::encode::write(writer, plugin_output).map_err(rmp_encode_err)
rmp_serde::encode::write_named(writer, plugin_output).map_err(rmp_encode_err)
}

fn decode(
Expand Down

0 comments on commit f2169c8

Please sign in to comment.