From 461f69ac5da82071822b354123e55a42871fd52b Mon Sep 17 00:00:00 2001
From: Devyn Cairns <devyn.cairns@gmail.com>
Date: Sun, 25 Feb 2024 13:49:10 -0800
Subject: [PATCH] Rename spans in the serialized form of Value (#11972)

[Discord
context](https://discord.com/channels/601130461678272522/615962413203718156/1211158641793695744)

<!--
if this PR closes one or more issues, you can automatically link the PR
with
them by using one of the [*linking
keywords*](https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue#linking-a-pull-request-to-an-issue-using-a-keyword),
e.g.
- this PR should close #xxxx
- fixes #xxxx

you can also mention related issues, PRs or discussions!
-->

# Description
<!--
Thank you for improving Nushell. Please, check our [contributing
guide](../CONTRIBUTING.md) and talk to the core team before making major
changes.

Description of your pull request goes here. **Provide examples and/or
screenshots** if your changes affect the user experience.
-->
Span fields were previously renamed to `internal_span` to discourage
their use in Rust code, but this change also affected the serde I/O for
Value. I don't believe the Python plugin was ever updated to reflect
this change.

This effectively changes it back, but just for the serialized form.
There are good reasons for doing this:

1. `internal_span` is a much longer name, and would be one of the most
common strings found in serialized Value data, probably bulking up the
plugin I/O

2. This change was never really meant to have implications for plugins,
and was just meant to be a hint that `.span()` should be used instead in
Rust code.

When Span refactoring is complete, the serialized form of Value will
probably change again in some significant way, so I think for now it's
best that it's left like this.

This has implications for #11911, particularly for documentation and for
the Python plugin as that was already updated in that PR to reflect
`internal_span`. If this is merged first, I will update that PR.

This would probably be considered a breaking change as it would break
plugin I/O compatibility (but not Rust code). I think it can probably go
in any major release though - all things considered, it's pretty minor,
and users are already expected to recompile plugins for new major
versions. However, it may also be worth holding off to do it together
with #11911 as that PR makes breaking changes in general a little bit
friendlier.

# User-Facing Changes
<!-- List of all changes that impact the user experience here. This
helps us keep track of breaking changes. -->

Requires plugin recompile.

# Tests + Formatting
<!--
Don't forget to add tests that cover your changes.

Make sure you've run and fixed any issues with these commands:

- `cargo fmt --all -- --check` to check standard code formatting (`cargo
fmt --all` applies these changes)
- `cargo clippy --workspace -- -D warnings -D clippy::unwrap_used` to
check that you're using the standard code style
- `cargo test --workspace` to check that all tests pass (on Windows make
sure to [enable developer
mode](https://learn.microsoft.com/en-us/windows/apps/get-started/developer-mode-features-and-debugging))
- `cargo run -- -c "use std testing; testing run-tests --path
crates/nu-std"` to run the tests for the standard library

> **Note**
> from `nushell` you can also use the `toolkit` as follows
> ```bash
> use toolkit.nu # or use an `env_change` hook to activate it
automatically
> toolkit check pr
> ```
-->
- :green_circle: `toolkit fmt`
- :green_circle: `toolkit clippy`
- :green_circle: `toolkit test`
- :green_circle: `toolkit test stdlib`

Nothing outside of `Value` itself had to be changed to make tests pass.
I did not check the Python plugin and whether it works now, but it was
broken before. It may work again as I think the main incompatibility it
had was expecting to use `span`

# After Submitting
<!-- If your PR had any user-facing changes, update [the
documentation](https://github.com/nushell/nushell.github.io) after the
PR is merged, if necessary. This will help us keep the docs up to date.
-->
---
 crates/nu-protocol/src/value/mod.rs | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/crates/nu-protocol/src/value/mod.rs b/crates/nu-protocol/src/value/mod.rs
index a8b06c4582..8333053d2c 100644
--- a/crates/nu-protocol/src/value/mod.rs
+++ b/crates/nu-protocol/src/value/mod.rs
@@ -49,48 +49,56 @@ pub enum Value {
         val: bool,
         // note: spans are being refactored out of Value
         // please use .span() instead of matching this span value
+        #[serde(rename = "span")]
         internal_span: Span,
     },
     Int {
         val: i64,
         // note: spans are being refactored out of Value
         // please use .span() instead of matching this span value
+        #[serde(rename = "span")]
         internal_span: Span,
     },
     Float {
         val: f64,
         // note: spans are being refactored out of Value
         // please use .span() instead of matching this span value
+        #[serde(rename = "span")]
         internal_span: Span,
     },
     Filesize {
         val: i64,
         // note: spans are being refactored out of Value
         // please use .span() instead of matching this span value
+        #[serde(rename = "span")]
         internal_span: Span,
     },
     Duration {
         val: i64,
         // note: spans are being refactored out of Value
         // please use .span() instead of matching this span value
+        #[serde(rename = "span")]
         internal_span: Span,
     },
     Date {
         val: DateTime<FixedOffset>,
         // note: spans are being refactored out of Value
         // please use .span() instead of matching this span value
+        #[serde(rename = "span")]
         internal_span: Span,
     },
     Range {
         val: Box<Range>,
         // note: spans are being refactored out of Value
         // please use .span() instead of matching this span value
+        #[serde(rename = "span")]
         internal_span: Span,
     },
     String {
         val: String,
         // note: spans are being refactored out of Value
         // please use .span() instead of matching this span value
+        #[serde(rename = "span")]
         internal_span: Span,
     },
     Glob {
@@ -98,53 +106,62 @@ pub enum Value {
         no_expand: bool,
         // note: spans are being refactored out of Value
         // please use .span() instead of matching this span value
+        #[serde(rename = "span")]
         internal_span: Span,
     },
     Record {
         val: Record,
         // note: spans are being refactored out of Value
         // please use .span() instead of matching this span value
+        #[serde(rename = "span")]
         internal_span: Span,
     },
     List {
         vals: Vec<Value>,
         // note: spans are being refactored out of Value
         // please use .span() instead of matching this span value
+        #[serde(rename = "span")]
         internal_span: Span,
     },
     Block {
         val: BlockId,
         // note: spans are being refactored out of Value
         // please use .span() instead of matching this span value
+        #[serde(rename = "span")]
         internal_span: Span,
     },
     Closure {
         val: Closure,
         // note: spans are being refactored out of Value
         // please use .span() instead of matching this span value
+        #[serde(rename = "span")]
         internal_span: Span,
     },
     Nothing {
         // note: spans are being refactored out of Value
         // please use .span() instead of matching this span value
+        #[serde(rename = "span")]
         internal_span: Span,
     },
     Error {
         error: Box<ShellError>,
         // note: spans are being refactored out of Value
         // please use .span() instead of matching this span value
+        #[serde(rename = "span")]
         internal_span: Span,
     },
     Binary {
         val: Vec<u8>,
         // note: spans are being refactored out of Value
         // please use .span() instead of matching this span value
+        #[serde(rename = "span")]
         internal_span: Span,
     },
     CellPath {
         val: CellPath,
         // note: spans are being refactored out of Value
         // please use .span() instead of matching this span value
+        #[serde(rename = "span")]
         internal_span: Span,
     },
     #[serde(skip_serializing)]
@@ -152,6 +169,7 @@ pub enum Value {
         val: Box<dyn CustomValue>,
         // note: spans are being refactored out of Value
         // please use .span() instead of matching this span value
+        #[serde(rename = "span")]
         internal_span: Span,
     },
     #[serde(skip)]