diff --git a/Cargo.lock b/Cargo.lock index 24fd67f90e..4902050017 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -593,7 +593,7 @@ dependencies = [ "http", "http-body", "hyper", - "itoa", + "itoa 1.0.6", "matchit", "memchr", "mime", @@ -3011,7 +3011,7 @@ checksum = "bd6effc99afb63425aff9b05836f029929e345a6148a14b7ecd5ab67af944482" dependencies = [ "bytes 1.4.0", "fnv", - "itoa", + "itoa 1.0.6", ] [[package]] @@ -3070,7 +3070,7 @@ dependencies = [ "http-body", "httparse", "httpdate", - "itoa", + "itoa 1.0.6", "pin-project-lite 0.2.9", "socket2", "tokio", @@ -3336,6 +3336,12 @@ dependencies = [ "either", ] +[[package]] +name = "itoa" +version = "0.4.8" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b71991ff56294aa922b450139ee08b3bfc70982c6b2c7562771375cf73542dd4" + [[package]] name = "itoa" version = "1.0.6" @@ -3396,12 +3402,6 @@ dependencies = [ "wasm-bindgen", ] -[[package]] -name = "json_comments" -version = "0.2.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "41ee439ee368ba4a77ac70d04f14015415af8600d6c894dc1f11bd79758c57d5" - [[package]] name = "jwt" version = "0.16.0" @@ -5667,7 +5667,7 @@ dependencies = [ "bitflags", "errno 0.2.8", "io-lifetimes 0.5.3", - "itoa", + "itoa 1.0.6", "libc", "linux-raw-sys 0.0.42", "once_cell", @@ -6099,7 +6099,19 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "057d394a50403bcac12672b2b18fb387ab6d289d957dab67dd201875391e52f1" dependencies = [ "indexmap", - "itoa", + "itoa 1.0.6", + "ryu", + "serde", +] + +[[package]] +name = "serde_json_lenient" +version = "0.1.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7d7b9ce5b0a63c6269b9623ed828b39259545a6ec0d8a35d6135ad6af6232add" +dependencies = [ + "indexmap", + "itoa 0.4.8", "ryu", "serde", ] @@ -6122,7 +6134,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "d3491c14715ca2294c4d6a88f15e84739788c1d030eed8c110436aafdaa2f3fd" dependencies = [ "form_urlencoded", - "itoa", + "itoa 1.0.6", "ryu", "serde", ] @@ -6148,7 +6160,7 @@ dependencies = [ "fs", "futures 0.3.28", "gpui", - "json_comments", + "indoc", "lazy_static", "postage", "pretty_assertions", @@ -6157,6 +6169,7 @@ dependencies = [ "serde", "serde_derive", "serde_json", + "serde_json_lenient", "smallvec", "sqlez", "staff_mode", @@ -6507,7 +6520,7 @@ dependencies = [ "hkdf", "hmac 0.12.1", "indexmap", - "itoa", + "itoa 1.0.6", "libc", "libsqlite3-sys", "log", @@ -6993,7 +7006,7 @@ version = "0.3.21" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "8f3403384eaacbca9923fa06940178ac13e4edb725486d70e8e15881d0c836cc" dependencies = [ - "itoa", + "itoa 1.0.6", "serde", "time-core", "time-macros", diff --git a/crates/gpui/src/app.rs b/crates/gpui/src/app.rs index 882800f128..41684f3226 100644 --- a/crates/gpui/src/app.rs +++ b/crates/gpui/src/app.rs @@ -445,7 +445,7 @@ type WindowBoundsCallback = Box>, &mut WindowContext) -> bool>; type ActiveLabeledTasksCallback = Box bool>; -type DeserializeActionCallback = fn(json: &str) -> anyhow::Result>; +type DeserializeActionCallback = fn(json: serde_json::Value) -> anyhow::Result>; type WindowShouldCloseSubscriptionCallback = Box bool>; pub struct AppContext { @@ -624,14 +624,14 @@ impl AppContext { pub fn deserialize_action( &self, name: &str, - argument: Option<&str>, + argument: Option, ) -> Result> { let callback = self .action_deserializers .get(name) .ok_or_else(|| anyhow!("unknown action {}", name))? .1; - callback(argument.unwrap_or("{}")) + callback(argument.unwrap_or_else(|| serde_json::Value::Object(Default::default()))) .with_context(|| format!("invalid data for action {}", name)) } @@ -5573,7 +5573,7 @@ mod tests { let action1 = cx .deserialize_action( "test::something::ComplexAction", - Some(r#"{"arg": "a", "count": 5}"#), + Some(serde_json::from_str(r#"{"arg": "a", "count": 5}"#).unwrap()), ) .unwrap(); let action2 = cx diff --git a/crates/gpui/src/app/action.rs b/crates/gpui/src/app/action.rs index 5b4df68a65..c6b43e489b 100644 --- a/crates/gpui/src/app/action.rs +++ b/crates/gpui/src/app/action.rs @@ -11,7 +11,7 @@ pub trait Action: 'static { fn qualified_name() -> &'static str where Self: Sized; - fn from_json_str(json: &str) -> anyhow::Result> + fn from_json_str(json: serde_json::Value) -> anyhow::Result> where Self: Sized; } @@ -38,7 +38,7 @@ macro_rules! actions { $crate::__impl_action! { $namespace, $name, - fn from_json_str(_: &str) -> $crate::anyhow::Result> { + fn from_json_str(_: $crate::serde_json::Value) -> $crate::anyhow::Result> { Ok(Box::new(Self)) } } @@ -58,8 +58,8 @@ macro_rules! impl_actions { $crate::__impl_action! { $namespace, $name, - fn from_json_str(json: &str) -> $crate::anyhow::Result> { - Ok(Box::new($crate::serde_json::from_str::(json)?)) + fn from_json_str(json: $crate::serde_json::Value) -> $crate::anyhow::Result> { + Ok(Box::new($crate::serde_json::from_value::(json)?)) } } )* diff --git a/crates/gpui/src/app/window.rs b/crates/gpui/src/app/window.rs index cfcef626df..cffce6c3a6 100644 --- a/crates/gpui/src/app/window.rs +++ b/crates/gpui/src/app/window.rs @@ -394,7 +394,7 @@ impl<'a> WindowContext<'a> { .iter() .filter_map(move |(name, (type_id, deserialize))| { if let Some(action_depth) = handler_depths_by_action_type.get(type_id).copied() { - let action = deserialize("{}").ok()?; + let action = deserialize(serde_json::Value::Object(Default::default())).ok()?; let bindings = self .keystroke_matcher .bindings_for_action_type(*type_id) diff --git a/crates/settings/Cargo.toml b/crates/settings/Cargo.toml index dab4b91992..b5dc301a5c 100644 --- a/crates/settings/Cargo.toml +++ b/crates/settings/Cargo.toml @@ -21,7 +21,7 @@ util = { path = "../util" } anyhow.workspace = true futures.workspace = true -json_comments = "0.2" +serde_json_lenient = {version = "0.1", features = ["preserve_order", "raw_value"]} lazy_static.workspace = true postage.workspace = true rust-embed.workspace = true @@ -37,6 +37,6 @@ tree-sitter-json = "*" [dev-dependencies] gpui = { path = "../gpui", features = ["test-support"] } fs = { path = "../fs", features = ["test-support"] } - +indoc.workspace = true pretty_assertions = "1.3.0" unindent.workspace = true diff --git a/crates/settings/src/keymap_file.rs b/crates/settings/src/keymap_file.rs index e607a254bd..d2e656ebe3 100644 --- a/crates/settings/src/keymap_file.rs +++ b/crates/settings/src/keymap_file.rs @@ -1,5 +1,5 @@ use crate::{settings_store::parse_json_with_comments, SettingsAssets}; -use anyhow::{Context, Result}; +use anyhow::{anyhow, Context, Result}; use collections::BTreeMap; use gpui::{keymap_matcher::Binding, AppContext}; use schemars::{ @@ -8,7 +8,7 @@ use schemars::{ JsonSchema, }; use serde::Deserialize; -use serde_json::{value::RawValue, Value}; +use serde_json::Value; use util::{asset_str, ResultExt}; #[derive(Deserialize, Default, Clone, JsonSchema)] @@ -24,7 +24,7 @@ pub struct KeymapBlock { #[derive(Deserialize, Default, Clone)] #[serde(transparent)] -pub struct KeymapAction(Box); +pub struct KeymapAction(Value); impl JsonSchema for KeymapAction { fn schema_name() -> String { @@ -37,11 +37,12 @@ impl JsonSchema for KeymapAction { } #[derive(Deserialize)] -struct ActionWithData(Box, Box); +struct ActionWithData(Box, Value); impl KeymapFile { pub fn load_asset(asset_path: &str, cx: &mut AppContext) -> Result<()> { let content = asset_str::(asset_path); + Self::parse(content.as_ref())?.add_to_cx(cx) } @@ -54,18 +55,27 @@ impl KeymapFile { let bindings = bindings .into_iter() .filter_map(|(keystroke, action)| { - let action = action.0.get(); + let action = action.0; // This is a workaround for a limitation in serde: serde-rs/json#497 // We want to deserialize the action data as a `RawValue` so that we can // deserialize the action itself dynamically directly from the JSON // string. But `RawValue` currently does not work inside of an untagged enum. - if action.starts_with('[') { - let ActionWithData(name, data) = serde_json::from_str(action).log_err()?; - cx.deserialize_action(&name, Some(data.get())) + if let Value::Array(items) = action { + let Ok([name, data]): Result<[serde_json::Value; 2], _> = items.try_into() else { + return Some(Err(anyhow!("Expected array of length 2"))); + }; + let serde_json::Value::String(name) = name else { + return Some(Err(anyhow!("Expected first item in array to be a string."))) + }; + cx.deserialize_action( + &name, + Some(data), + ) + } else if let Value::String(name) = action { + cx.deserialize_action(&name, None) } else { - let name = serde_json::from_str(action).log_err()?; - cx.deserialize_action(name, None) + return Some(Err(anyhow!("Expected two-element array, got {:?}", action))); } .with_context(|| { format!( @@ -118,3 +128,24 @@ impl KeymapFile { serde_json::to_value(root_schema).unwrap() } } + +#[cfg(test)] +mod tests { + use crate::KeymapFile; + + #[test] + fn can_deserialize_keymap_with_trailing_comma() { + let json = indoc::indoc! {"[ + // Standard macOS bindings + { + \"bindings\": { + \"up\": \"menu::SelectPrev\", + }, + }, + ] + " + + }; + KeymapFile::parse(json).unwrap(); + } +} diff --git a/crates/settings/src/settings_store.rs b/crates/settings/src/settings_store.rs index 39c6a2c122..1188018cd8 100644 --- a/crates/settings/src/settings_store.rs +++ b/crates/settings/src/settings_store.rs @@ -834,11 +834,8 @@ fn to_pretty_json(value: &impl Serialize, indent_size: usize, indent_prefix_len: } pub fn parse_json_with_comments(content: &str) -> Result { - Ok(serde_json::from_reader( - json_comments::CommentSettings::c_style().strip_comments(content.as_bytes()), - )?) + Ok(serde_json_lenient::from_str(content)?) } - #[cfg(test)] mod tests { use super::*;