From dea126c2968ec4cfd56a8fc179d056e1537de0bf Mon Sep 17 00:00:00 2001 From: Jean Mertz Date: Sun, 1 Feb 2026 07:14:54 +0100 Subject: [PATCH] enhance(config): allow boolean and numeric values for styles Users can now use booleans and numbers for specific configuration fields, providing a more concise way to configure the assistant's behavior. For instance, `inline_results` now accepts `true` (full), `false` (off), or a number to specify line truncation. Similarly, `link_style` fields now accept `true` for full links and `false` to disable them. This change also renames the `is_default` property to `discard_when_merged` within `MergeableString` and `MergeableVec`. This new name more accurately describes the property's role: ensuring that initial "default" configurations are completely replaced when a user provides their own settings, regardless of the chosen merge strategy. Finally, `serde-untagged` has been added to the workspace dependencies, to allow for better error messages when deserializing untagged enums. This has not been applied to all enums, but will be done in a future commit. Signed-off-by: Jean Mertz --- Cargo.lock | 13 ++ Cargo.toml | 1 + crates/jp_config/Cargo.toml | 1 + crates/jp_config/src/assistant.rs | 56 +++--- .../jp_config/src/conversation/tool/style.rs | 180 +++++++++++++++++- crates/jp_config/src/internal/merge/string.rs | 122 ++++++------ crates/jp_config/src/internal/merge/vec.rs | 110 +++++------ crates/jp_config/src/partial.rs | 4 +- ...ts__partial_app_config_default_values.snap | 6 +- crates/jp_config/src/style.rs | 64 ++++++- crates/jp_config/src/types/string.rs | 58 ++++-- crates/jp_config/src/types/vec.rs | 74 ++++--- crates/jp_config/src/util.rs | 4 +- 13 files changed, 501 insertions(+), 192 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index d3548f70..c212175e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2202,6 +2202,7 @@ dependencies = [ "relative-path", "schematic", "serde", + "serde-untagged", "serde_json", "serde_json5", "serde_yaml", @@ -3918,6 +3919,18 @@ dependencies = [ "serde_derive", ] +[[package]] +name = "serde-untagged" +version = "0.1.9" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f9faf48a4a2d2693be24c6289dbe26552776eb7737074e6722891fadbe6c5058" +dependencies = [ + "erased-serde", + "serde", + "serde_core", + "typeid", +] + [[package]] name = "serde_core" version = "1.0.226" diff --git a/Cargo.toml b/Cargo.toml index dca1f734..3b5a3945 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -90,6 +90,7 @@ schemars = { version = "1.0.0-alpha.17", default-features = false } schemars_08 = { package = "schemars", version = "0.8.22", default-features = false } schematic = { git = "https://github.com/JeanMertz/schematic", branch = "merged", default-features = false } serde = { version = "1", default-features = false } +serde-untagged = { version = "0.1", default-features = false } serde_json = { version = "1", default-features = false } serde_json5 = { version = "0.2", default-features = false } serde_yaml = { version = "0.9", default-features = false } diff --git a/crates/jp_config/Cargo.toml b/crates/jp_config/Cargo.toml index 0ba75a87..6220c0d3 100644 --- a/crates/jp_config/Cargo.toml +++ b/crates/jp_config/Cargo.toml @@ -43,6 +43,7 @@ schematic = { workspace = true, features = [ "type_url", ] } serde = { workspace = true, features = ["derive"] } +serde-untagged = { workspace = true } serde_json = { workspace = true, features = ["preserve_order"] } serde_json5 = { workspace = true } serde_yaml = { workspace = true } diff --git a/crates/jp_config/src/assistant.rs b/crates/jp_config/src/assistant.rs index a31c251b..1fbc8be7 100644 --- a/crates/jp_config/src/assistant.rs +++ b/crates/jp_config/src/assistant.rs @@ -22,7 +22,7 @@ use crate::{ partial::{ToPartial, partial_opt, partial_opt_config, partial_opts}, types::{ string::{MergeableString, PartialMergeableString, PartialMergedString}, - vec::{MergeableVec, MergedVec, MergedVecStrategy}, + vec::{MergeableVec, MergedVec}, }, }; @@ -102,8 +102,8 @@ impl ToPartial for AssistantConfig { #[expect(clippy::trivially_copy_pass_by_ref, clippy::unnecessary_wraps)] fn default_instructions(_: &()) -> TransformResult> { Ok(MergeableVec::Merged(MergedVec { - strategy: MergedVecStrategy::Replace, - is_default: true, + strategy: None, + discard_when_merged: true, value: vec![PartialInstructionsConfig { title: Some("How to respond to the user".into()), items: Some(vec![ @@ -130,7 +130,7 @@ fn default_system_prompt(_: &()) -> TransformResult`. This - // means `is_default` is left untouched. This is *correct*, but - // it might be confusing in some cases, so we might want to - // change this in the future. - is_default: true, + // means `discard_when_merged` is left untouched. This is + // *correct*, but it might be confusing in some cases, so we + // might want to change this in the future. + discard_when_merged: true, }) ); @@ -188,7 +188,7 @@ mod tests { assert_eq!( p.instructions, MergeableVec::Merged(MergedVec { - strategy: MergedVecStrategy::Replace, + strategy: None, value: vec![ PartialInstructionsConfig { title: Some("foo".into()), @@ -200,7 +200,7 @@ mod tests { ..Default::default() } ], - is_default: true, + discard_when_merged: true, }) ); @@ -209,7 +209,7 @@ mod tests { assert_eq!( p.instructions, MergeableVec::Merged(MergedVec { - strategy: MergedVecStrategy::Replace, + strategy: None, value: vec![ PartialInstructionsConfig { title: Some("foo".into()), @@ -225,7 +225,7 @@ mod tests { ..Default::default() } ], - is_default: true, + discard_when_merged: true, }) ); @@ -234,12 +234,12 @@ mod tests { assert_eq!( p.instructions, MergeableVec::Merged(MergedVec { - strategy: MergedVecStrategy::Replace, + strategy: None, value: vec![PartialInstructionsConfig { title: Some("qux".into()), ..Default::default() }], - is_default: true, + discard_when_merged: true, }) ); @@ -248,12 +248,12 @@ mod tests { assert_eq!( p.instructions, MergeableVec::Merged(MergedVec { - strategy: MergedVecStrategy::Replace, + strategy: None, value: vec![PartialInstructionsConfig { title: Some("boop".into()), ..Default::default() }], - is_default: true, + discard_when_merged: true, }) ); @@ -265,13 +265,13 @@ mod tests { assert_eq!( p.instructions, MergeableVec::Merged(MergedVec { - strategy: MergedVecStrategy::Replace, + strategy: None, value: vec![PartialInstructionsConfig { title: Some("quux".into()), items: Some(vec!["one".into()]), ..Default::default() }], - is_default: true, + discard_when_merged: true, }) ); @@ -280,13 +280,13 @@ mod tests { assert_eq!( p.instructions, MergeableVec::Merged(MergedVec { - strategy: MergedVecStrategy::Replace, + strategy: None, value: vec![PartialInstructionsConfig { title: Some("quux".into()), items: Some(vec!["two".into()]), ..Default::default() }], - is_default: true, + discard_when_merged: true, }) ); @@ -311,7 +311,7 @@ mod tests { value: Some("foo".into()), strategy: None, separator: None, - is_default: None, + discard_when_merged: None, })) ); @@ -325,7 +325,7 @@ mod tests { value: Some("foo".into()), strategy: Some(MergedStringStrategy::Append), separator: None, - is_default: None, + discard_when_merged: None, })) ); @@ -341,7 +341,7 @@ mod tests { value: Some("foo".into()), strategy: Some(MergedStringStrategy::Append), separator: Some(MergedStringSeparator::Space), - is_default: None, + discard_when_merged: None, })) ); } @@ -441,7 +441,7 @@ mod tests { items: None, examples: vec![], }], - strategy: MergedVecStrategy::Append, + strategy: Some(MergedVecStrategy::Append), ..Default::default() } .into(), @@ -465,7 +465,7 @@ mod tests { examples: vec![], }, ], - strategy: MergedVecStrategy::Append, + strategy: Some(MergedVecStrategy::Append), ..Default::default() } .into(), @@ -555,7 +555,7 @@ mod tests { value: Some("foo".into()), strategy: Some(MergedStringStrategy::Append), separator: Some(MergedStringSeparator::Paragraph), - is_default: None, + discard_when_merged: None, })), instructions: MergedVec { value: vec![ @@ -574,7 +574,7 @@ mod tests { examples: vec![], }, ], - strategy: MergedVecStrategy::Append, + strategy: Some(MergedVecStrategy::Append), ..Default::default() } .into(), diff --git a/crates/jp_config/src/conversation/tool/style.rs b/crates/jp_config/src/conversation/tool/style.rs index f7205fa5..ced2fc65 100644 --- a/crates/jp_config/src/conversation/tool/style.rs +++ b/crates/jp_config/src/conversation/tool/style.rs @@ -71,7 +71,7 @@ impl ToPartial for DisplayStyleConfig { /// Even if disabled or truncated, a link will be added to a file containing the /// full tool call results. Additionally, the full tool call results will be /// sent back to the assistant, regardless of this setting. -#[derive(Debug, Clone, PartialEq, Serialize, Deserialize, ConfigEnum)] +#[derive(Debug, Clone, PartialEq, Serialize, ConfigEnum)] #[serde(rename_all = "snake_case")] pub enum InlineResults { /// Never show the tool call results inline. @@ -91,6 +91,90 @@ impl Default for InlineResults { } } +impl<'de> Deserialize<'de> for InlineResults { + fn deserialize(deserializer: D) -> Result + where + D: serde::Deserializer<'de>, + { + struct InlineResultsVisitor; + + impl<'de> serde::de::Visitor<'de> for InlineResultsVisitor { + type Value = InlineResults; + + fn expecting(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { + formatter.write_str( + "a boolean, a string (\"off\", \"full\"), or a number for truncation", + ) + } + + fn visit_bool(self, v: bool) -> Result + where + E: serde::de::Error, + { + if v { + Ok(InlineResults::Full) + } else { + Ok(InlineResults::Off) + } + } + + fn visit_str(self, v: &str) -> Result + where + E: serde::de::Error, + { + match v { + "off" => Ok(InlineResults::Off), + "full" => Ok(InlineResults::Full), + s => s + .parse::() + .map(|lines| InlineResults::Truncate(TruncateLines { lines })) + .map_err(|_| { + serde::de::Error::unknown_variant(v, &["off", "full", "a number"]) + }), + } + } + + fn visit_u64(self, v: u64) -> Result + where + E: serde::de::Error, + { + usize::try_from(v) + .map(|lines| InlineResults::Truncate(TruncateLines { lines })) + .map_err(|_| { + serde::de::Error::invalid_value( + serde::de::Unexpected::Unsigned(v), + &"a number", + ) + }) + } + + fn visit_map(self, map: A) -> Result + where + A: serde::de::MapAccess<'de>, + { + // Reuse the derived deserializer for the complex case (Truncate object) + #[derive(Deserialize)] + #[serde(rename_all = "snake_case")] + enum Helper { + Off, + Full, + Truncate(TruncateLines), + } + + let helper = + Helper::deserialize(serde::de::value::MapAccessDeserializer::new(map))?; + match helper { + Helper::Off => Ok(InlineResults::Off), + Helper::Full => Ok(InlineResults::Full), + Helper::Truncate(t) => Ok(InlineResults::Truncate(t)), + } + } + } + + deserializer.deserialize_any(InlineResultsVisitor) + } +} + /// Truncate the tool call results to the first N lines. #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] pub struct TruncateLines { @@ -119,7 +203,7 @@ impl fmt::Display for TruncateLines { } /// How to display the link to the file containing the tool call results. -#[derive(Debug, Clone, PartialEq, Default, Serialize, Deserialize, ConfigEnum)] +#[derive(Debug, Clone, PartialEq, Default, Serialize, ConfigEnum)] #[serde(rename_all = "lowercase")] pub enum LinkStyle { /// Full (raw) link. @@ -133,6 +217,50 @@ pub enum LinkStyle { Off, } +impl<'de> Deserialize<'de> for LinkStyle { + fn deserialize(deserializer: D) -> Result + where + D: serde::Deserializer<'de>, + { + struct LinkStyleVisitor; + + impl serde::de::Visitor<'_> for LinkStyleVisitor { + type Value = LinkStyle; + + fn expecting(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { + formatter.write_str("a boolean or a string (\"off\", \"full\", \"osc8\")") + } + + fn visit_bool(self, v: bool) -> Result + where + E: serde::de::Error, + { + if v { + Ok(LinkStyle::Full) + } else { + Ok(LinkStyle::Off) + } + } + + fn visit_str(self, v: &str) -> Result + where + E: serde::de::Error, + { + match v { + "off" => Ok(LinkStyle::Off), + "full" => Ok(LinkStyle::Full), + "osc8" => Ok(LinkStyle::Osc8), + _ => Err(serde::de::Error::unknown_variant(v, &[ + "off", "full", "osc8", + ])), + } + } + } + + deserializer.deserialize_any(LinkStyleVisitor) + } +} + /// Define the name to serialize and deserialize for a unit variant. mod strings { use crate::named_unit_variant; @@ -182,3 +310,51 @@ impl FromStr for ParametersStyle { }) } } + +#[cfg(test)] +mod tests { + use serde_json::from_str; + + use super::*; + + #[test] + fn test_link_style_deserialization() { + assert_eq!(from_str::("false").unwrap(), LinkStyle::Off); + assert_eq!(from_str::("true").unwrap(), LinkStyle::Full); + assert_eq!(from_str::("\"off\"").unwrap(), LinkStyle::Off); + assert_eq!(from_str::("\"full\"").unwrap(), LinkStyle::Full); + assert_eq!(from_str::("\"osc8\"").unwrap(), LinkStyle::Osc8); + } + + #[test] + fn test_inline_results_deserialization() { + assert_eq!( + from_str::("false").unwrap(), + InlineResults::Off + ); + assert_eq!( + from_str::("true").unwrap(), + InlineResults::Full + ); + assert_eq!( + from_str::("\"off\"").unwrap(), + InlineResults::Off + ); + assert_eq!( + from_str::("\"full\"").unwrap(), + InlineResults::Full + ); + assert_eq!( + from_str::("10").unwrap(), + InlineResults::Truncate(TruncateLines { lines: 10 }) + ); + assert_eq!( + from_str::("\"25\"").unwrap(), + InlineResults::Truncate(TruncateLines { lines: 25 }) + ); + assert_eq!( + from_str::(r#"{"truncate": {"lines": 5}}"#).unwrap(), + InlineResults::Truncate(TruncateLines { lines: 5 }) + ); + } +} diff --git a/crates/jp_config/src/internal/merge/string.rs b/crates/jp_config/src/internal/merge/string.rs index c9173f63..8c022e91 100644 --- a/crates/jp_config/src/internal/merge/string.rs +++ b/crates/jp_config/src/internal/merge/string.rs @@ -13,7 +13,7 @@ pub fn string_with_strategy( _context: &(), ) -> MergeResult { // If prev is default, replace regardless of strategy. - if prev.is_default() { + if prev.discard_when_merged() { return Ok(Some(next)); } @@ -23,11 +23,13 @@ pub fn string_with_strategy( }; let next_is_replace = matches!(next, PartialMergeableString::String(_)); - let (strategy, separator, next_value, is_default) = match next { + let (strategy, separator, next_value, discard_when_merged) = match next { PartialMergeableString::String(v) => { (Some(MergedStringStrategy::Replace), None, Some(v), None) } - PartialMergeableString::Merged(v) => (v.strategy, v.separator, v.value, v.is_default), + PartialMergeableString::Merged(v) => { + (v.strategy, v.separator, v.value, v.discard_when_merged) + } }; let sep = separator.as_ref().map_or("", |sep| sep.as_str()); @@ -51,7 +53,7 @@ pub fn string_with_strategy( value, strategy, separator, - is_default, + discard_when_merged, }) })) } @@ -83,13 +85,13 @@ mod tests { value: Some("bar".to_owned()), strategy: Some(MergedStringStrategy::Append), separator: Some(MergedStringSeparator::None), - is_default: None, + discard_when_merged: None, }), expected: PartialMergeableString::Merged(PartialMergedString { value: Some("foobar".to_owned()), strategy: Some(MergedStringStrategy::Append), separator: Some(MergedStringSeparator::None), - is_default: None, + discard_when_merged: None, }), }, TestCase { @@ -98,13 +100,13 @@ mod tests { value: Some("bar".to_owned()), strategy: Some(MergedStringStrategy::Replace), separator: Some(MergedStringSeparator::None), - is_default: None, + discard_when_merged: None, }), expected: PartialMergeableString::Merged(PartialMergedString { value: Some("bar".to_owned()), strategy: Some(MergedStringStrategy::Replace), separator: Some(MergedStringSeparator::None), - is_default: None, + discard_when_merged: None, }), }, TestCase { @@ -112,7 +114,7 @@ mod tests { value: Some("foo".to_owned()), strategy: Some(MergedStringStrategy::Append), separator: Some(MergedStringSeparator::None), - is_default: None, + discard_when_merged: None, }), next: PartialMergeableString::String("bar".to_owned()), expected: PartialMergeableString::String("bar".to_owned()), @@ -122,19 +124,19 @@ mod tests { value: Some("foo".to_owned()), strategy: Some(MergedStringStrategy::Append), separator: Some(MergedStringSeparator::None), - is_default: None, + discard_when_merged: None, }), next: PartialMergeableString::Merged(PartialMergedString { value: Some("bar".to_owned()), strategy: Some(MergedStringStrategy::Replace), separator: Some(MergedStringSeparator::None), - is_default: None, + discard_when_merged: None, }), expected: PartialMergeableString::Merged(PartialMergedString { value: Some("bar".to_owned()), strategy: Some(MergedStringStrategy::Replace), separator: Some(MergedStringSeparator::None), - is_default: None, + discard_when_merged: None, }), }, TestCase { @@ -142,19 +144,19 @@ mod tests { value: Some("foo".to_owned()), strategy: Some(MergedStringStrategy::Append), separator: Some(MergedStringSeparator::None), - is_default: None, + discard_when_merged: None, }), next: PartialMergeableString::Merged(PartialMergedString { value: Some("bar".to_owned()), strategy: Some(MergedStringStrategy::Replace), separator: Some(MergedStringSeparator::None), - is_default: None, + discard_when_merged: None, }), expected: PartialMergeableString::Merged(PartialMergedString { value: Some("bar".to_owned()), strategy: Some(MergedStringStrategy::Replace), separator: Some(MergedStringSeparator::None), - is_default: None, + discard_when_merged: None, }), }, TestCase { @@ -162,39 +164,39 @@ mod tests { value: Some("foo".to_owned()), strategy: Some(MergedStringStrategy::Append), separator: Some(MergedStringSeparator::None), - is_default: None, + discard_when_merged: None, }), next: PartialMergeableString::Merged(PartialMergedString { value: Some("bar".to_owned()), strategy: Some(MergedStringStrategy::Append), separator: Some(MergedStringSeparator::None), - is_default: None, + discard_when_merged: None, }), expected: PartialMergeableString::Merged(PartialMergedString { value: Some("foobar".to_owned()), strategy: Some(MergedStringStrategy::Append), separator: Some(MergedStringSeparator::None), - is_default: None, + discard_when_merged: None, }), }, TestCase { prev: PartialMergeableString::Merged(PartialMergedString { value: Some("foo".to_owned()), strategy: Some(MergedStringStrategy::Append), - is_default: None, + discard_when_merged: None, separator: Some(MergedStringSeparator::None), }), next: PartialMergeableString::Merged(PartialMergedString { value: Some("bar".to_owned()), strategy: Some(MergedStringStrategy::Replace), separator: Some(MergedStringSeparator::None), - is_default: None, + discard_when_merged: None, }), expected: PartialMergeableString::Merged(PartialMergedString { value: Some("bar".to_owned()), strategy: Some(MergedStringStrategy::Replace), separator: Some(MergedStringSeparator::None), - is_default: None, + discard_when_merged: None, }), }, TestCase { @@ -203,13 +205,13 @@ mod tests { value: Some("bar".to_owned()), strategy: Some(MergedStringStrategy::Append), separator: Some(MergedStringSeparator::Space), - is_default: None, + discard_when_merged: None, }), expected: PartialMergeableString::Merged(PartialMergedString { value: Some("foo bar".to_owned()), strategy: Some(MergedStringStrategy::Append), separator: Some(MergedStringSeparator::Space), - is_default: None, + discard_when_merged: None, }), }, TestCase { @@ -218,13 +220,13 @@ mod tests { value: Some("bar".to_owned()), strategy: Some(MergedStringStrategy::Append), separator: Some(MergedStringSeparator::Line), - is_default: None, + discard_when_merged: None, }), expected: PartialMergeableString::Merged(PartialMergedString { value: Some("foo\nbar".to_owned()), strategy: Some(MergedStringStrategy::Append), separator: Some(MergedStringSeparator::Line), - is_default: None, + discard_when_merged: None, }), }, TestCase { @@ -233,13 +235,13 @@ mod tests { value: Some("bar".to_owned()), strategy: Some(MergedStringStrategy::Append), separator: Some(MergedStringSeparator::Paragraph), - is_default: None, + discard_when_merged: None, }), expected: PartialMergeableString::Merged(PartialMergedString { value: Some("foo\n\nbar".to_owned()), strategy: Some(MergedStringStrategy::Append), separator: Some(MergedStringSeparator::Paragraph), - is_default: None, + discard_when_merged: None, }), }, ]; @@ -275,13 +277,13 @@ mod tests { value: Some("bar".to_owned()), strategy: Some(MergedStringStrategy::Prepend), separator: Some(MergedStringSeparator::None), - is_default: None, + discard_when_merged: None, }), expected: PartialMergeableString::Merged(PartialMergedString { value: Some("barfoo".to_owned()), strategy: Some(MergedStringStrategy::Prepend), separator: Some(MergedStringSeparator::None), - is_default: None, + discard_when_merged: None, }), }, TestCase { @@ -290,13 +292,13 @@ mod tests { value: Some("bar".to_owned()), strategy: Some(MergedStringStrategy::Replace), separator: Some(MergedStringSeparator::None), - is_default: None, + discard_when_merged: None, }), expected: PartialMergeableString::Merged(PartialMergedString { value: Some("bar".to_owned()), strategy: Some(MergedStringStrategy::Replace), separator: Some(MergedStringSeparator::None), - is_default: None, + discard_when_merged: None, }), }, TestCase { @@ -304,7 +306,7 @@ mod tests { value: Some("foo".to_owned()), strategy: Some(MergedStringStrategy::Prepend), separator: Some(MergedStringSeparator::None), - is_default: None, + discard_when_merged: None, }), next: PartialMergeableString::String("bar".to_owned()), expected: PartialMergeableString::String("bar".to_owned()), @@ -314,19 +316,19 @@ mod tests { value: Some("foo".to_owned()), strategy: Some(MergedStringStrategy::Prepend), separator: Some(MergedStringSeparator::None), - is_default: None, + discard_when_merged: None, }), next: PartialMergeableString::Merged(PartialMergedString { value: Some("bar".to_owned()), strategy: Some(MergedStringStrategy::Replace), separator: Some(MergedStringSeparator::None), - is_default: None, + discard_when_merged: None, }), expected: PartialMergeableString::Merged(PartialMergedString { value: Some("bar".to_owned()), strategy: Some(MergedStringStrategy::Replace), separator: Some(MergedStringSeparator::None), - is_default: None, + discard_when_merged: None, }), }, TestCase { @@ -334,19 +336,19 @@ mod tests { value: Some("foo".to_owned()), strategy: Some(MergedStringStrategy::Prepend), separator: Some(MergedStringSeparator::None), - is_default: None, + discard_when_merged: None, }), next: PartialMergeableString::Merged(PartialMergedString { value: Some("bar".to_owned()), strategy: Some(MergedStringStrategy::Replace), separator: Some(MergedStringSeparator::None), - is_default: None, + discard_when_merged: None, }), expected: PartialMergeableString::Merged(PartialMergedString { value: Some("bar".to_owned()), strategy: Some(MergedStringStrategy::Replace), separator: Some(MergedStringSeparator::None), - is_default: None, + discard_when_merged: None, }), }, TestCase { @@ -354,19 +356,19 @@ mod tests { value: Some("foo".to_owned()), strategy: Some(MergedStringStrategy::Prepend), separator: Some(MergedStringSeparator::None), - is_default: None, + discard_when_merged: None, }), next: PartialMergeableString::Merged(PartialMergedString { value: Some("bar".to_owned()), strategy: Some(MergedStringStrategy::Prepend), separator: Some(MergedStringSeparator::None), - is_default: None, + discard_when_merged: None, }), expected: PartialMergeableString::Merged(PartialMergedString { value: Some("barfoo".to_owned()), strategy: Some(MergedStringStrategy::Prepend), separator: Some(MergedStringSeparator::None), - is_default: None, + discard_when_merged: None, }), }, TestCase { @@ -374,19 +376,19 @@ mod tests { value: Some("foo".to_owned()), strategy: Some(MergedStringStrategy::Prepend), separator: Some(MergedStringSeparator::None), - is_default: None, + discard_when_merged: None, }), next: PartialMergeableString::Merged(PartialMergedString { value: Some("bar".to_owned()), strategy: Some(MergedStringStrategy::Replace), separator: Some(MergedStringSeparator::None), - is_default: None, + discard_when_merged: None, }), expected: PartialMergeableString::Merged(PartialMergedString { value: Some("bar".to_owned()), strategy: Some(MergedStringStrategy::Replace), separator: Some(MergedStringSeparator::None), - is_default: None, + discard_when_merged: None, }), }, TestCase { @@ -395,13 +397,13 @@ mod tests { value: Some("bar".to_owned()), strategy: Some(MergedStringStrategy::Prepend), separator: Some(MergedStringSeparator::Space), - is_default: None, + discard_when_merged: None, }), expected: PartialMergeableString::Merged(PartialMergedString { value: Some("bar foo".to_owned()), strategy: Some(MergedStringStrategy::Prepend), separator: Some(MergedStringSeparator::Space), - is_default: None, + discard_when_merged: None, }), }, TestCase { @@ -410,13 +412,13 @@ mod tests { value: Some("bar".to_owned()), strategy: Some(MergedStringStrategy::Prepend), separator: Some(MergedStringSeparator::Line), - is_default: None, + discard_when_merged: None, }), expected: PartialMergeableString::Merged(PartialMergedString { value: Some("bar\nfoo".to_owned()), strategy: Some(MergedStringStrategy::Prepend), separator: Some(MergedStringSeparator::Line), - is_default: None, + discard_when_merged: None, }), }, TestCase { @@ -425,13 +427,13 @@ mod tests { value: Some("bar".to_owned()), strategy: Some(MergedStringStrategy::Prepend), separator: Some(MergedStringSeparator::Paragraph), - is_default: None, + discard_when_merged: None, }), expected: PartialMergeableString::Merged(PartialMergedString { value: Some("bar\n\nfoo".to_owned()), strategy: Some(MergedStringStrategy::Prepend), separator: Some(MergedStringSeparator::Paragraph), - is_default: None, + discard_when_merged: None, }), }, ]; @@ -461,7 +463,7 @@ mod tests { value: Some("foo".to_owned()), strategy: None, separator: None, - is_default: Some(true), + discard_when_merged: Some(true), }), next: PartialMergeableString::String("bar".to_owned()), expected: PartialMergeableString::String("bar".to_owned()), @@ -471,19 +473,19 @@ mod tests { value: Some("foo".to_owned()), strategy: None, separator: None, - is_default: Some(true), + discard_when_merged: Some(true), }), next: PartialMergeableString::Merged(PartialMergedString { value: Some("bar".to_owned()), strategy: Some(MergedStringStrategy::Prepend), separator: Some(MergedStringSeparator::None), - is_default: None, + discard_when_merged: None, }), expected: PartialMergeableString::Merged(PartialMergedString { value: Some("bar".to_owned()), strategy: Some(MergedStringStrategy::Prepend), separator: Some(MergedStringSeparator::None), - is_default: None, + discard_when_merged: None, }), }), ("default stacking", TestCase { @@ -491,19 +493,19 @@ mod tests { value: Some("foo".to_owned()), strategy: None, separator: None, - is_default: Some(true), + discard_when_merged: Some(true), }), next: PartialMergeableString::Merged(PartialMergedString { value: Some("foo".to_owned()), strategy: Some(MergedStringStrategy::Prepend), separator: Some(MergedStringSeparator::None), - is_default: Some(true), + discard_when_merged: Some(true), }), expected: PartialMergeableString::Merged(PartialMergedString { value: Some("foo".to_owned()), strategy: Some(MergedStringStrategy::Prepend), separator: Some(MergedStringSeparator::None), - is_default: Some(true), + discard_when_merged: Some(true), }), }), ("next as default", TestCase { @@ -511,19 +513,19 @@ mod tests { value: Some("bar".to_owned()), strategy: None, separator: None, - is_default: Some(false), + discard_when_merged: Some(false), }), next: PartialMergeableString::Merged(PartialMergedString { value: Some("foo".to_owned()), strategy: Some(MergedStringStrategy::Prepend), separator: Some(MergedStringSeparator::None), - is_default: Some(true), + discard_when_merged: Some(true), }), expected: PartialMergeableString::Merged(PartialMergedString { value: Some("foobar".to_owned()), strategy: Some(MergedStringStrategy::Prepend), separator: Some(MergedStringSeparator::None), - is_default: Some(true), + discard_when_merged: Some(true), }), }), ]; diff --git a/crates/jp_config/src/internal/merge/vec.rs b/crates/jp_config/src/internal/merge/vec.rs index a9fe51c6..ee4ffa2f 100644 --- a/crates/jp_config/src/internal/merge/vec.rs +++ b/crates/jp_config/src/internal/merge/vec.rs @@ -17,7 +17,7 @@ where T: Clone + PartialEq + Serialize + DeserializeOwned + Schematic, { // If prev is default, replace regardless of strategy. - if prev.is_default() { + if prev.discard_when_merged() { return Ok(Some(next)); } @@ -27,24 +27,24 @@ where }; let next_is_merged = matches!(next, MergeableVec::Merged(_)); - let (strategy, mut next_value, is_default) = match next { - MergeableVec::Vec(v) => (MergedVecStrategy::Append, v, false), - MergeableVec::Merged(v) => (v.strategy, v.value, v.is_default), + let (strategy, mut next_value, discard_when_merged) = match next { + MergeableVec::Vec(v) => (None, v, false), + MergeableVec::Merged(v) => (v.strategy, v.value, v.discard_when_merged), }; let value = match strategy { - MergedVecStrategy::Append => { + None | Some(MergedVecStrategy::Append) => { prev_value.append(&mut next_value); prev_value } - MergedVecStrategy::Replace => next_value, + Some(MergedVecStrategy::Replace) => next_value, }; Ok(Some(if next_is_merged { MergeableVec::Merged(MergedVec { value, strategy, - is_default, + discard_when_merged, }) } else { MergeableVec::Vec(value) @@ -76,33 +76,33 @@ mod tests { prev: MergeableVec::Vec(vec![1, 2, 3]), next: MergeableVec::Merged(MergedVec { value: vec![4, 5, 6], - strategy: MergedVecStrategy::Append, - is_default: false, + strategy: Some(MergedVecStrategy::Append), + discard_when_merged: false, }), expected: MergeableVec::Merged(MergedVec { value: vec![1, 2, 3, 4, 5, 6], - strategy: MergedVecStrategy::Append, - is_default: false, + strategy: Some(MergedVecStrategy::Append), + discard_when_merged: false, }), }, TestCase { prev: MergeableVec::Vec(vec![1, 2, 3]), next: MergeableVec::Merged(MergedVec { value: vec![4, 5, 6], - strategy: MergedVecStrategy::Replace, - is_default: false, + strategy: Some(MergedVecStrategy::Replace), + discard_when_merged: false, }), expected: MergeableVec::Merged(MergedVec { value: vec![4, 5, 6], - strategy: MergedVecStrategy::Replace, - is_default: false, + strategy: Some(MergedVecStrategy::Replace), + discard_when_merged: false, }), }, TestCase { prev: MergeableVec::Merged(MergedVec { value: vec![1, 2, 3], - strategy: MergedVecStrategy::Append, - is_default: false, + strategy: Some(MergedVecStrategy::Append), + discard_when_merged: false, }), next: MergeableVec::Vec(vec![4, 5, 6]), expected: MergeableVec::Vec(vec![1, 2, 3, 4, 5, 6]), @@ -110,52 +110,52 @@ mod tests { TestCase { prev: MergeableVec::Merged(MergedVec { value: vec![1, 2, 3], - strategy: MergedVecStrategy::Append, - is_default: false, + strategy: Some(MergedVecStrategy::Append), + discard_when_merged: false, }), next: MergeableVec::Merged(MergedVec { value: vec![4, 5, 6], - strategy: MergedVecStrategy::Replace, - is_default: false, + strategy: Some(MergedVecStrategy::Replace), + discard_when_merged: false, }), expected: MergeableVec::Merged(MergedVec { value: vec![4, 5, 6], - strategy: MergedVecStrategy::Replace, - is_default: false, + strategy: Some(MergedVecStrategy::Replace), + discard_when_merged: false, }), }, TestCase { prev: MergeableVec::Merged(MergedVec { value: vec![1, 2, 3], - strategy: MergedVecStrategy::Append, - is_default: false, + strategy: Some(MergedVecStrategy::Append), + discard_when_merged: false, }), next: MergeableVec::Merged(MergedVec { value: vec![4, 5, 6], - strategy: MergedVecStrategy::Append, - is_default: false, + strategy: Some(MergedVecStrategy::Append), + discard_when_merged: false, }), expected: MergeableVec::Merged(MergedVec { value: vec![1, 2, 3, 4, 5, 6], - strategy: MergedVecStrategy::Append, - is_default: false, + strategy: Some(MergedVecStrategy::Append), + discard_when_merged: false, }), }, TestCase { prev: MergeableVec::Merged(MergedVec { value: vec![1, 2, 3], - strategy: MergedVecStrategy::Append, - is_default: false, + strategy: Some(MergedVecStrategy::Append), + discard_when_merged: false, }), next: MergeableVec::Merged(MergedVec { value: vec![4, 5, 6], - strategy: MergedVecStrategy::Replace, - is_default: false, + strategy: Some(MergedVecStrategy::Replace), + discard_when_merged: false, }), expected: MergeableVec::Merged(MergedVec { value: vec![4, 5, 6], - strategy: MergedVecStrategy::Replace, - is_default: false, + strategy: Some(MergedVecStrategy::Replace), + discard_when_merged: false, }), }, ]; @@ -183,8 +183,8 @@ mod tests { ("default with next string", TestCase { prev: MergeableVec::Merged(MergedVec { value: vec![1], - strategy: MergedVecStrategy::Append, - is_default: true, + strategy: Some(MergedVecStrategy::Append), + discard_when_merged: true, }), next: MergeableVec::Vec(vec![2]), expected: MergeableVec::Vec(vec![2]), @@ -192,52 +192,52 @@ mod tests { ("default does not merge", TestCase { prev: MergeableVec::Merged(MergedVec { value: vec![1], - strategy: MergedVecStrategy::Append, - is_default: true, + strategy: Some(MergedVecStrategy::Append), + discard_when_merged: true, }), next: MergeableVec::Merged(MergedVec { value: vec![2], - strategy: MergedVecStrategy::Append, - is_default: false, + strategy: Some(MergedVecStrategy::Append), + discard_when_merged: false, }), expected: MergeableVec::Merged(MergedVec { value: vec![2], - strategy: MergedVecStrategy::Append, - is_default: false, + strategy: Some(MergedVecStrategy::Append), + discard_when_merged: false, }), }), ("default stacking", TestCase { prev: MergeableVec::Merged(MergedVec { value: vec![1], - strategy: MergedVecStrategy::Append, - is_default: true, + strategy: Some(MergedVecStrategy::Append), + discard_when_merged: true, }), next: MergeableVec::Merged(MergedVec { value: vec![2], - strategy: MergedVecStrategy::Append, - is_default: true, + strategy: Some(MergedVecStrategy::Append), + discard_when_merged: true, }), expected: MergeableVec::Merged(MergedVec { value: vec![2], - strategy: MergedVecStrategy::Append, - is_default: true, + strategy: Some(MergedVecStrategy::Append), + discard_when_merged: true, }), }), ("next as default", TestCase { prev: MergeableVec::Merged(MergedVec { value: vec![1], - strategy: MergedVecStrategy::Append, - is_default: false, + strategy: Some(MergedVecStrategy::Append), + discard_when_merged: false, }), next: MergeableVec::Merged(MergedVec { value: vec![2], - strategy: MergedVecStrategy::Append, - is_default: true, + strategy: Some(MergedVecStrategy::Append), + discard_when_merged: true, }), expected: MergeableVec::Merged(MergedVec { value: vec![1, 2], - strategy: MergedVecStrategy::Append, - is_default: true, + strategy: Some(MergedVecStrategy::Append), + discard_when_merged: true, }), }), ]; diff --git a/crates/jp_config/src/partial.rs b/crates/jp_config/src/partial.rs index e1bc0892..73af641b 100644 --- a/crates/jp_config/src/partial.rs +++ b/crates/jp_config/src/partial.rs @@ -3,9 +3,9 @@ use schematic::Config; /// Convert a configuration into a partial configuration. -pub trait ToPartial: Config { +pub trait ToPartial::Partial>: Config { /// Convert a configuration into a partial configuration. - fn to_partial(&self) -> Self::Partial; + fn to_partial(&self) -> Partial; } /// Get the current or default value, if any. diff --git a/crates/jp_config/src/snapshots/jp_config__tests__partial_app_config_default_values.snap b/crates/jp_config/src/snapshots/jp_config__tests__partial_app_config_default_values.snap index 9c7cfe19..58c9bcb0 100644 --- a/crates/jp_config/src/snapshots/jp_config__tests__partial_app_config_default_values.snap +++ b/crates/jp_config/src/snapshots/jp_config__tests__partial_app_config_default_values.snap @@ -28,7 +28,7 @@ Ok( ), strategy: None, separator: None, - is_default: Some( + discard_when_merged: Some( true, ), }, @@ -55,8 +55,8 @@ Ok( examples: [], }, ], - strategy: Replace, - is_default: true, + strategy: None, + discard_when_merged: true, }, ), tool_choice: None, diff --git a/crates/jp_config/src/style.rs b/crates/jp_config/src/style.rs index 4f37cb3d..e662418c 100644 --- a/crates/jp_config/src/style.rs +++ b/crates/jp_config/src/style.rs @@ -5,6 +5,8 @@ pub mod reasoning; pub mod tool_call; pub mod typewriter; +use std::fmt; + use schematic::{Config, ConfigEnum}; use serde::{Deserialize, Serialize}; @@ -79,7 +81,7 @@ impl ToPartial for StyleConfig { } /// Formatting style for links. -#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, Default, Serialize, Deserialize, ConfigEnum)] +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, Default, Serialize, ConfigEnum)] #[serde(rename_all = "lowercase")] pub enum LinkStyle { /// No link. @@ -90,3 +92,63 @@ pub enum LinkStyle { #[default] Osc8, } + +impl<'de> Deserialize<'de> for LinkStyle { + fn deserialize(deserializer: D) -> Result + where + D: serde::Deserializer<'de>, + { + struct LinkStyleVisitor; + + impl serde::de::Visitor<'_> for LinkStyleVisitor { + type Value = LinkStyle; + + fn expecting(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { + formatter.write_str("a boolean or a string (\"off\", \"full\", \"osc8\")") + } + + fn visit_bool(self, v: bool) -> Result + where + E: serde::de::Error, + { + if v { + Ok(LinkStyle::Full) + } else { + Ok(LinkStyle::Off) + } + } + + fn visit_str(self, v: &str) -> Result + where + E: serde::de::Error, + { + match v { + "off" => Ok(LinkStyle::Off), + "full" => Ok(LinkStyle::Full), + "osc8" => Ok(LinkStyle::Osc8), + _ => Err(serde::de::Error::unknown_variant(v, &[ + "off", "full", "osc8", + ])), + } + } + } + + deserializer.deserialize_any(LinkStyleVisitor) + } +} + +#[cfg(test)] +mod tests { + use serde_json::from_str; + + use super::*; + + #[test] + fn test_link_style_deserialization() { + assert_eq!(from_str::("false").unwrap(), LinkStyle::Off); + assert_eq!(from_str::("true").unwrap(), LinkStyle::Full); + assert_eq!(from_str::("\"off\"").unwrap(), LinkStyle::Off); + assert_eq!(from_str::("\"full\"").unwrap(), LinkStyle::Full); + assert_eq!(from_str::("\"osc8\"").unwrap(), LinkStyle::Osc8); + } +} diff --git a/crates/jp_config/src/types/string.rs b/crates/jp_config/src/types/string.rs index 2e17a1c9..0d24323d 100644 --- a/crates/jp_config/src/types/string.rs +++ b/crates/jp_config/src/types/string.rs @@ -3,18 +3,19 @@ use std::{convert::Infallible, ops::Deref, str::FromStr}; use schematic::{Config, ConfigEnum, PartialConfig as _}; -use serde::{Deserialize, Serialize}; +use serde::{Deserialize, Deserializer, Serialize}; +use serde_untagged::UntaggedEnumVisitor; use crate::{ assignment::{AssignKeyValue, AssignResult, KvAssignment, missing_key}, - delta::PartialConfigDelta, + delta::{PartialConfigDelta, delta_opt}, partial::ToPartial, }; /// String value, either defaulting to a merge strategy of `replace`, or /// defining a specific merge strategy. #[derive(Debug, Clone, PartialEq, Config)] -#[config(serde(untagged))] +#[config(serde(untagged), no_deserialize_derive)] pub enum MergeableString { /// A string that is merged using the [`schematic::merge::replace`] #[setting(default)] @@ -28,8 +29,8 @@ pub enum MergeableString { impl PartialMergeableString { /// Returns `true` if the `MergeableString` is the default value. #[must_use] - pub fn is_default(&self) -> bool { - matches!(self, Self::Merged(v) if v.is_default.is_some_and(|v| v)) + pub fn discard_when_merged(&self) -> bool { + matches!(self, Self::Merged(v) if v.discard_when_merged.is_some_and(|v| v)) } } @@ -89,11 +90,11 @@ impl AssignKeyValue for PartialMergeableString { impl PartialConfigDelta for PartialMergeableString { fn delta(&self, next: Self) -> Self { - if self == &next { - return Self::empty(); + match (self, next) { + (Self::Merged(prev), Self::Merged(next)) => Self::Merged(prev.delta(next)), + (Self::String(prev), Self::String(next)) if prev == &next => Self::empty(), + (_, next) => next, } - - next } } @@ -106,6 +107,18 @@ impl ToPartial for MergeableString { } } +impl<'de> Deserialize<'de> for PartialMergeableString { + fn deserialize(deserializer: D) -> Result + where + D: Deserializer<'de>, + { + UntaggedEnumVisitor::new() + .string(|v| Ok(Self::String(v.to_owned()))) + .map(|map| map.deserialize().map(Self::Merged)) + .deserialize(deserializer) + } +} + /// Strings that are merged using the specified merge strategy. #[derive(Debug, Clone, PartialEq, Config)] #[config(rename_all = "snake_case")] @@ -122,12 +135,13 @@ pub struct MergedString { #[setting(default)] pub separator: MergedStringSeparator, - /// Whether the value is the default value. + /// Whether the value is discarded when another value is merged in, + /// regardless of the merge strategy of the other value. /// - /// When `true`, if another value is merged in, this value will be - /// overwritten. + /// This is useful for "default" values that should only be used when no + /// other value is set. #[setting(default)] - pub is_default: bool, + pub discard_when_merged: bool, } impl AssignKeyValue for PartialMergedString { @@ -137,7 +151,7 @@ impl AssignKeyValue for PartialMergedString { "value" => self.value = kv.try_some_string()?, "strategy" => self.strategy = kv.try_some_from_str()?, "separator" => self.separator = kv.try_some_from_str()?, - "is_default" => self.is_default = kv.try_some_bool()?, + "discard_when_merged" => self.discard_when_merged = kv.try_some_bool()?, _ => return missing_key(&kv), } @@ -151,7 +165,21 @@ impl ToPartial for MergedString { value: Some(self.value.clone()), strategy: Some(self.strategy), separator: Some(self.separator), - is_default: Some(self.is_default), + discard_when_merged: Some(self.discard_when_merged), + } + } +} + +impl PartialConfigDelta for PartialMergedString { + fn delta(&self, next: Self) -> Self { + Self { + value: delta_opt(self.value.as_ref(), next.value), + strategy: delta_opt(self.strategy.as_ref(), next.strategy), + separator: delta_opt(self.separator.as_ref(), next.separator), + discard_when_merged: delta_opt( + self.discard_when_merged.as_ref(), + next.discard_when_merged, + ), } } } diff --git a/crates/jp_config/src/types/vec.rs b/crates/jp_config/src/types/vec.rs index 8bd269fd..b7d08a70 100644 --- a/crates/jp_config/src/types/vec.rs +++ b/crates/jp_config/src/types/vec.rs @@ -3,7 +3,8 @@ use std::ops::{Deref, DerefMut}; use schematic::{Config, ConfigEnum, PartialConfig as _, Schematic}; -use serde::{Deserialize, Serialize, de::DeserializeOwned}; +use serde::{Deserialize, Deserializer, Serialize, de::DeserializeOwned}; +use serde_untagged::UntaggedEnumVisitor; use crate::{delta::PartialConfigDelta, partial::ToPartial}; @@ -32,7 +33,7 @@ use crate::{delta::PartialConfigDelta, partial::ToPartial}; /// /// [`AssignKeyValue`]: crate::AssignKeyValue /// [`MergeableString`]: super::string::MergeableString -#[derive(Debug, Config, Clone, PartialEq, Serialize, Deserialize)] +#[derive(Debug, Clone, PartialEq, Serialize, Config)] #[serde(untagged, rename_all = "snake_case")] pub enum MergeableVec { /// A vec that is merged using the [`schematic::merge::append_vec`] @@ -63,6 +64,21 @@ impl DerefMut for MergeableVec { } } +impl<'de, T> Deserialize<'de> for MergeableVec +where + T: Clone + DeserializeOwned, +{ + fn deserialize(deserializer: D) -> Result + where + D: Deserializer<'de>, + { + UntaggedEnumVisitor::new() + .seq(|v| v.deserialize().map(MergeableVec::Vec)) + .map(|map| map.deserialize().map(MergeableVec::Merged)) + .deserialize(deserializer) + } +} + impl MergeableVec { /// Consumes the `MergeableVec` and returns the underlying [`Vec`]. #[must_use] @@ -84,27 +100,29 @@ impl MergeableVec { /// Returns `true` if the `MergeableVec` is the default value. #[must_use] - pub const fn is_default(&self) -> bool { - matches!(self, Self::Merged(v) if v.is_default) + pub const fn discard_when_merged(&self) -> bool { + matches!(self, Self::Merged(v) if v.discard_when_merged) + } + + /// Get a mutable reference to the [`MergedVec`], if applicable. + pub const fn as_merged_mut(&mut self) -> Option<&mut MergedVec> { + match self { + Self::Vec(_) => None, + Self::Merged(v) => Some(v), + } } } -impl MergeableVec { - /// Convert the `MergeableVec` into a [`MergeableVec`] of `T:Partial`. - /// - /// Note that we do not implement `ToPartial` for `MergeableVec` because - /// that trait requires `to_partial` to return a `PartialConfig`, which is - /// not what we want in this case. - /// - /// For more details, see [`MergeableVec`]. - #[must_use] - pub fn to_partial(&self) -> MergeableVec { +impl + ToPartial> for MergeableVec +{ + fn to_partial(&self) -> MergeableVec { match self { Self::Vec(v) => MergeableVec::Vec(v.iter().map(ToPartial::to_partial).collect()), Self::Merged(v) => MergeableVec::Merged(MergedVec { value: v.value.iter().map(ToPartial::to_partial).collect(), strategy: v.strategy, - is_default: v.is_default, + discard_when_merged: v.discard_when_merged, }), } } @@ -184,7 +202,7 @@ where } /// Strings that are merged using the specified merge strategy. -#[derive(Debug, Config, Default, Clone, PartialEq, Serialize, Deserialize)] +#[derive(Debug, Default, Clone, PartialEq, Serialize, Deserialize, Config)] #[serde(rename_all = "snake_case")] pub struct MergedVec { /// The vec value. @@ -192,16 +210,24 @@ pub struct MergedVec { pub value: Vec, /// The merge strategy. - #[setting(default)] - pub strategy: MergedVecStrategy, + #[setting(default, skip_serializing_if = "Option::is_none")] + #[serde(default, skip_serializing_if = "Option::is_none")] + // The strategy is wrapped in an `Option`, because otherwise + // `PartialMergedVec` would always set `strategy` to the default value, + // which is not what we want. Anyway, this isn't great, and it's related to + // the fact that `MergedVec` behaves "special" compared to `MergedString` in + // `schematic`, which isn't great either, but it's wrapped in tests, so if + // you change this you'll see what happens. + pub strategy: Option, - /// Whether the value is the default value. + /// Whether the value is discarded when another value is merged in, + /// regardless of the merge strategy of the other value. /// - /// When `true`, if another value is merged in, this value will be - /// overwritten. + /// This is useful for "default" values that should only be used when no + /// other value is set. #[setting(default)] #[serde(default)] - pub is_default: bool, + pub discard_when_merged: bool, } impl From> for MergeableVec { @@ -217,8 +243,8 @@ where fn to_partial(&self) -> Self::Partial { Self::Partial { value: Some(self.value.clone()), - strategy: Some(self.strategy), - is_default: Some(self.is_default), + strategy: self.strategy, + discard_when_merged: Some(self.discard_when_merged), } } } diff --git a/crates/jp_config/src/util.rs b/crates/jp_config/src/util.rs index 56a28f9a..10f09e64 100644 --- a/crates/jp_config/src/util.rs +++ b/crates/jp_config/src/util.rs @@ -584,8 +584,8 @@ mod tests { examples: vec![], }, ], - strategy: MergedVecStrategy::Replace, - is_default: false, + strategy: Some(MergedVecStrategy::Replace), + discard_when_merged: false, } .into();