From 992359a1910975013ca5d3e0fccee7ed1993e7cc Mon Sep 17 00:00:00 2001 From: Devyn Cairns Date: Mon, 18 Mar 2024 05:34:21 -0700 Subject: [PATCH] Support for custom values in plugin examples (#12213) # Description @ayax79 says that the dataframe commands all have dataframe custom values in their examples, and they're used for tests. Rather than send the custom values to the engine, if they're in examples, this change just renders them using `to_base_value()` first. That way we avoid potentially having to hold onto custom values in `plugins.nu` that might not be valid indefinitely - as will be the case for dataframes in particular - but we still avoid forcing plugin writers to not use custom values in their examples. # User-Facing Changes - Custom values usable in plugin examples # Tests + Formatting - :green_circle: `toolkit fmt` - :green_circle: `toolkit clippy` - :green_circle: `toolkit test` - :green_circle: `toolkit test stdlib` # After Submitting --- .../nu-plugin/src/plugin/interface/engine.rs | 12 ++++- .../src/plugin/interface/engine/tests.rs | 44 ++++++++++++++++- .../src/protocol/plugin_custom_value.rs | 47 +++++++++++++++++++ .../nu_plugin_custom_values/src/generate.rs | 7 ++- .../nu_plugin_custom_values/src/generate2.rs | 14 +++++- crates/nu_plugin_custom_values/src/update.rs | 14 +++++- tests/plugins/custom_values.rs | 14 ++++++ 7 files changed, 147 insertions(+), 5 deletions(-) diff --git a/crates/nu-plugin/src/plugin/interface/engine.rs b/crates/nu-plugin/src/plugin/interface/engine.rs index c4ebf7abfd..547c57a1f8 100644 --- a/crates/nu-plugin/src/plugin/interface/engine.rs +++ b/crates/nu-plugin/src/plugin/interface/engine.rs @@ -406,10 +406,20 @@ impl EngineInterface { } /// Write a call response of plugin signatures. + /// + /// Any custom values in the examples will be rendered using `to_base_value()`. pub(crate) fn write_signature( &self, - signature: Vec, + mut signature: Vec, ) -> Result<(), ShellError> { + // Render any custom values in the examples to plain values so that the engine doesn't + // have to keep custom values around just to render the help pages. + for sig in signature.iter_mut() { + for value in sig.examples.iter_mut().flat_map(|e| e.result.as_mut()) { + PluginCustomValue::render_to_base_value_in(value)?; + } + } + let response = PluginCallResponse::Signature(signature); self.write(PluginOutput::CallResponse(self.context()?, response))?; self.flush() diff --git a/crates/nu-plugin/src/plugin/interface/engine/tests.rs b/crates/nu-plugin/src/plugin/interface/engine/tests.rs index 372f3adf7a..599a135cc9 100644 --- a/crates/nu-plugin/src/plugin/interface/engine/tests.rs +++ b/crates/nu-plugin/src/plugin/interface/engine/tests.rs @@ -5,7 +5,7 @@ use std::{ use nu_protocol::{ engine::Closure, Config, CustomValue, IntoInterruptiblePipelineData, PipelineData, - PluginSignature, ShellError, Span, Spanned, Value, + PluginExample, PluginSignature, ShellError, Span, Spanned, Value, }; use crate::{ @@ -789,6 +789,48 @@ fn interface_write_signature() -> Result<(), ShellError> { Ok(()) } +#[test] +fn interface_write_signature_custom_value() -> Result<(), ShellError> { + let test = TestCase::new(); + let interface = test.engine().interface_for_context(38); + let signatures = vec![PluginSignature::build("test command").plugin_examples(vec![ + PluginExample { + example: "test command".into(), + description: "a test".into(), + result: Some(Value::test_custom_value(Box::new( + expected_test_custom_value(), + ))), + }, + ])]; + interface.write_signature(signatures.clone())?; + + let written = test.next_written().expect("nothing written"); + + match written { + PluginOutput::CallResponse(id, response) => { + assert_eq!(38, id, "id"); + match response { + PluginCallResponse::Signature(sigs) => { + assert_eq!(1, sigs.len(), "sigs.len"); + + let sig = &sigs[0]; + assert_eq!(1, sig.examples.len(), "sig.examples.len"); + + assert_eq!( + Some(Value::test_int(expected_test_custom_value().0 as i64)), + sig.examples[0].result, + ); + } + _ => panic!("unexpected response: {response:?}"), + } + } + _ => panic!("unexpected message written: {written:?}"), + } + + assert!(!test.has_unconsumed_write()); + Ok(()) +} + #[test] fn interface_write_engine_call_registers_subscription() -> Result<(), ShellError> { let mut manager = TestCase::new().engine(); diff --git a/crates/nu-plugin/src/protocol/plugin_custom_value.rs b/crates/nu-plugin/src/protocol/plugin_custom_value.rs index d426f655c3..158806ed97 100644 --- a/crates/nu-plugin/src/protocol/plugin_custom_value.rs +++ b/crates/nu-plugin/src/protocol/plugin_custom_value.rs @@ -367,6 +367,53 @@ impl PluginCustomValue { } }) } + + /// Render any custom values in the `Value` using `to_base_value()` + pub(crate) fn render_to_base_value_in(value: &mut Value) -> Result<(), ShellError> { + let span = value.span(); + match value { + Value::CustomValue { ref val, .. } => { + *value = val.to_base_value(span)?; + Ok(()) + } + // Any values that can contain other values need to be handled recursively + Value::Range { ref mut val, .. } => { + Self::render_to_base_value_in(&mut val.from)?; + Self::render_to_base_value_in(&mut val.to)?; + Self::render_to_base_value_in(&mut val.incr) + } + Value::Record { ref mut val, .. } => val + .iter_mut() + .try_for_each(|(_, rec_value)| Self::render_to_base_value_in(rec_value)), + Value::List { ref mut vals, .. } => { + vals.iter_mut().try_for_each(Self::render_to_base_value_in) + } + Value::Closure { ref mut val, .. } => val + .captures + .iter_mut() + .map(|(_, captured_value)| captured_value) + .try_for_each(Self::render_to_base_value_in), + // All of these don't contain other values + Value::Bool { .. } + | Value::Int { .. } + | Value::Float { .. } + | Value::Filesize { .. } + | Value::Duration { .. } + | Value::Date { .. } + | Value::String { .. } + | Value::Glob { .. } + | Value::Block { .. } + | Value::Nothing { .. } + | Value::Error { .. } + | Value::Binary { .. } + | Value::CellPath { .. } => Ok(()), + // Collect any lazy records that exist and try again + Value::LazyRecord { val, .. } => { + *value = val.collect()?; + Self::render_to_base_value_in(value) + } + } + } } impl Drop for PluginCustomValue { diff --git a/crates/nu_plugin_custom_values/src/generate.rs b/crates/nu_plugin_custom_values/src/generate.rs index 1d679bb98d..072af434a6 100644 --- a/crates/nu_plugin_custom_values/src/generate.rs +++ b/crates/nu_plugin_custom_values/src/generate.rs @@ -1,7 +1,7 @@ use crate::{cool_custom_value::CoolCustomValue, CustomValuePlugin}; use nu_plugin::{EngineInterface, EvaluatedCall, LabeledError, SimplePluginCommand}; -use nu_protocol::{Category, PluginSignature, Value}; +use nu_protocol::{Category, PluginExample, PluginSignature, Span, Value}; pub struct Generate; @@ -12,6 +12,11 @@ impl SimplePluginCommand for Generate { PluginSignature::build("custom-value generate") .usage("PluginSignature for a plugin that generates a custom value") .category(Category::Experimental) + .plugin_examples(vec![PluginExample { + example: "custom-value generate".into(), + description: "Generate a new CoolCustomValue".into(), + result: Some(CoolCustomValue::new("abc").into_value(Span::test_data())), + }]) } fn run( diff --git a/crates/nu_plugin_custom_values/src/generate2.rs b/crates/nu_plugin_custom_values/src/generate2.rs index 2c070226f5..a29e91b4a8 100644 --- a/crates/nu_plugin_custom_values/src/generate2.rs +++ b/crates/nu_plugin_custom_values/src/generate2.rs @@ -1,7 +1,7 @@ use crate::{second_custom_value::SecondCustomValue, CustomValuePlugin}; use nu_plugin::{EngineInterface, EvaluatedCall, LabeledError, SimplePluginCommand}; -use nu_protocol::{Category, PluginSignature, SyntaxShape, Value}; +use nu_protocol::{Category, PluginExample, PluginSignature, Span, SyntaxShape, Value}; pub struct Generate2; @@ -17,6 +17,18 @@ impl SimplePluginCommand for Generate2 { "An optional closure to pass the custom value to", ) .category(Category::Experimental) + .plugin_examples(vec![ + PluginExample { + example: "custom-value generate2".into(), + description: "Generate a new SecondCustomValue".into(), + result: Some(SecondCustomValue::new("xyz").into_value(Span::test_data())), + }, + PluginExample { + example: "custom-value generate2 { print }".into(), + description: "Generate a new SecondCustomValue and pass it to a closure".into(), + result: None, + }, + ]) } fn run( diff --git a/crates/nu_plugin_custom_values/src/update.rs b/crates/nu_plugin_custom_values/src/update.rs index 35c04473aa..a6d2e5e4bf 100644 --- a/crates/nu_plugin_custom_values/src/update.rs +++ b/crates/nu_plugin_custom_values/src/update.rs @@ -3,7 +3,7 @@ use crate::{ }; use nu_plugin::{EngineInterface, EvaluatedCall, LabeledError, SimplePluginCommand}; -use nu_protocol::{Category, PluginSignature, ShellError, Value}; +use nu_protocol::{Category, PluginExample, PluginSignature, ShellError, Span, Value}; pub struct Update; @@ -14,6 +14,18 @@ impl SimplePluginCommand for Update { PluginSignature::build("custom-value update") .usage("PluginSignature for a plugin that updates a custom value") .category(Category::Experimental) + .plugin_examples(vec![ + PluginExample { + example: "custom-value generate | custom-value update".into(), + description: "Update a CoolCustomValue".into(), + result: Some(CoolCustomValue::new("abcxyz").into_value(Span::test_data())), + }, + PluginExample { + example: "custom-value generate | custom-value update".into(), + description: "Update a SecondCustomValue".into(), + result: Some(CoolCustomValue::new("xyzabc").into_value(Span::test_data())), + }, + ]) } fn run( diff --git a/tests/plugins/custom_values.rs b/tests/plugins/custom_values.rs index dd93a55e9f..784e9c09af 100644 --- a/tests/plugins/custom_values.rs +++ b/tests/plugins/custom_values.rs @@ -180,3 +180,17 @@ fn drop_check_custom_value_prints_message_on_drop() { assert_eq!(actual.err, "DropCheckValue was dropped: Hello\n"); assert!(actual.status.success()); } + +#[test] +fn custom_value_in_example_is_rendered() { + let actual = nu_with_plugins!( + cwd: "tests", + plugin: ("nu_plugin_custom_values"), + "custom-value generate --help" + ); + + assert!(actual + .out + .contains("I used to be a custom value! My data was (abc)")); + assert!(actual.status.success()); +}