Allow comments in setting and keymap JSON files

Max Brunsfeld created

Change summary

Cargo.lock                                   | 20 ++++++++++++++++++--
crates/settings/Cargo.toml                   |  1 +
crates/settings/src/keymap_file.rs           | 11 ++++++-----
crates/settings/src/settings.rs              | 10 ++++++++--
crates/vim/src/vim_test_context.rs           |  2 +-
crates/zed/Cargo.toml                        |  2 +-
crates/zed/src/languages/json/highlights.scm |  2 ++
crates/zed/src/main.rs                       |  4 ++--
crates/zed/src/settings_file.rs              | 11 +++++++----
crates/zed/src/zed.rs                        |  2 +-
10 files changed, 47 insertions(+), 18 deletions(-)

Detailed changes

Cargo.lock 🔗

@@ -2572,6 +2572,12 @@ 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 = "json_env_logger"
 version = "0.1.1"
@@ -2643,7 +2649,7 @@ dependencies = [
  "text",
  "theme",
  "tree-sitter",
- "tree-sitter-json",
+ "tree-sitter-json 0.19.0",
  "tree-sitter-rust",
  "unindent",
  "util",
@@ -4304,6 +4310,7 @@ dependencies = [
  "assets",
  "collections",
  "gpui",
+ "json_comments",
  "schemars",
  "serde",
  "serde_json",
@@ -5193,6 +5200,15 @@ dependencies = [
  "tree-sitter",
 ]
 
+[[package]]
+name = "tree-sitter-json"
+version = "0.20.0"
+source = "git+https://github.com/tree-sitter/tree-sitter-json?rev=137e1ce6a02698fc246cdb9c6b886ed1de9a1ed8#137e1ce6a02698fc246cdb9c6b886ed1de9a1ed8"
+dependencies = [
+ "cc",
+ "tree-sitter",
+]
+
 [[package]]
 name = "tree-sitter-markdown"
 version = "0.0.1"
@@ -5837,7 +5853,7 @@ dependencies = [
  "toml",
  "tree-sitter",
  "tree-sitter-c",
- "tree-sitter-json",
+ "tree-sitter-json 0.20.0",
  "tree-sitter-markdown",
  "tree-sitter-rust",
  "tree-sitter-typescript",

crates/settings/Cargo.toml 🔗

@@ -17,6 +17,7 @@ gpui = { path = "../gpui" }
 theme = { path = "../theme" }
 util = { path = "../util" }
 anyhow = "1.0.38"
+json_comments = "0.2"
 schemars = "0.8"
 serde = { version = "1", features = ["derive", "rc"] }
 serde_json = { version = "1.0.64", features = ["preserve_order"] }

crates/settings/src/keymap_file.rs 🔗

@@ -1,3 +1,4 @@
+use crate::parse_json_with_comments;
 use anyhow::{Context, Result};
 use assets::Assets;
 use collections::BTreeMap;
@@ -7,14 +8,14 @@ use serde_json::value::RawValue;
 
 #[derive(Deserialize, Default, Clone)]
 #[serde(transparent)]
-pub struct KeymapFile(BTreeMap<String, ActionsByKeystroke>);
+pub struct KeymapFileContent(BTreeMap<String, ActionsByKeystroke>);
 
 type ActionsByKeystroke = BTreeMap<String, Box<RawValue>>;
 
 #[derive(Deserialize)]
-struct ActionWithData<'a>(#[serde(borrow)] &'a str, #[serde(borrow)] &'a RawValue);
+struct ActionWithData(Box<str>, Box<RawValue>);
 
-impl KeymapFile {
+impl KeymapFileContent {
     pub fn load_defaults(cx: &mut MutableAppContext) {
         for path in ["keymaps/default.json", "keymaps/vim.json"] {
             Self::load(path, cx).unwrap();
@@ -24,7 +25,7 @@ impl KeymapFile {
     pub fn load(asset_path: &str, cx: &mut MutableAppContext) -> Result<()> {
         let content = Assets::get(asset_path).unwrap().data;
         let content_str = std::str::from_utf8(content.as_ref()).unwrap();
-        Ok(serde_json::from_str::<Self>(content_str)?.add(cx)?)
+        Ok(parse_json_with_comments::<Self>(content_str)?.add(cx)?)
     }
 
     pub fn add(self, cx: &mut MutableAppContext) -> Result<()> {
@@ -42,7 +43,7 @@ impl KeymapFile {
                         // string. But `RawValue` currently does not work inside of an untagged enum.
                         let action = if action.starts_with('[') {
                             let ActionWithData(name, data) = serde_json::from_str(action)?;
-                            cx.deserialize_action(name, Some(data.get()))
+                            cx.deserialize_action(&name, Some(data.get()))
                         } else {
                             let name = serde_json::from_str(action)?;
                             cx.deserialize_action(name, None)

crates/settings/src/settings.rs 🔗

@@ -9,13 +9,13 @@ use schemars::{
     },
     JsonSchema,
 };
-use serde::Deserialize;
+use serde::{de::DeserializeOwned, Deserialize};
 use serde_json::Value;
 use std::{collections::HashMap, sync::Arc};
 use theme::{Theme, ThemeRegistry};
 use util::ResultExt as _;
 
-pub use keymap_file::KeymapFile;
+pub use keymap_file::KeymapFileContent;
 
 #[derive(Clone)]
 pub struct Settings {
@@ -260,3 +260,9 @@ fn merge_option<T: Copy>(target: &mut Option<T>, value: Option<T>) {
         *target = value;
     }
 }
+
+pub fn parse_json_with_comments<T: DeserializeOwned>(content: &str) -> Result<T> {
+    Ok(serde_json::from_reader(
+        json_comments::CommentSettings::c_style().strip_comments(content.as_bytes()),
+    )?)
+}

crates/vim/src/vim_test_context.rs 🔗

@@ -24,7 +24,7 @@ impl<'a> VimTestContext<'a> {
             editor::init(cx);
             crate::init(cx);
 
-            settings::KeymapFile::load("keymaps/vim.json", cx).unwrap();
+            settings::KeymapFileContent::load("keymaps/vim.json", cx).unwrap();
         });
 
         let params = cx.update(WorkspaceParams::test);

crates/zed/Cargo.toml 🔗

@@ -86,7 +86,7 @@ tiny_http = "0.8"
 toml = "0.5"
 tree-sitter = "0.20.4"
 tree-sitter-c = "0.20.1"
-tree-sitter-json = "0.19.0"
+tree-sitter-json = { git = "https://github.com/tree-sitter/tree-sitter-json", rev = "137e1ce6a02698fc246cdb9c6b886ed1de9a1ed8" }
 tree-sitter-rust = "0.20.1"
 tree-sitter-markdown = { git = "https://github.com/MDeiml/tree-sitter-markdown", rev = "330ecab87a3e3a7211ac69bbadc19eabecdb1cca" }
 tree-sitter-typescript = "0.20.1"

crates/zed/src/main.rs 🔗

@@ -17,7 +17,7 @@ use gpui::{App, AssetSource, AsyncAppContext, Task};
 use log::LevelFilter;
 use parking_lot::Mutex;
 use project::Fs;
-use settings::{self, KeymapFile, Settings, SettingsFileContent};
+use settings::{self, KeymapFileContent, Settings, SettingsFileContent};
 use smol::process::Command;
 use std::{env, fs, path::PathBuf, sync::Arc, thread, time::Duration};
 use theme::{ThemeRegistry, DEFAULT_THEME_NAME};
@@ -309,7 +309,7 @@ fn load_config_files(
     fs: Arc<dyn Fs>,
 ) -> oneshot::Receiver<(
     WatchedJsonFile<SettingsFileContent>,
-    WatchedJsonFile<KeymapFile>,
+    WatchedJsonFile<KeymapFileContent>,
 )> {
     let executor = app.background();
     let (tx, rx) = oneshot::channel();

crates/zed/src/settings_file.rs 🔗

@@ -4,7 +4,7 @@ use postage::sink::Sink as _;
 use postage::{prelude::Stream, watch};
 use project::Fs;
 use serde::Deserialize;
-use settings::{KeymapFile, Settings, SettingsFileContent};
+use settings::{parse_json_with_comments, KeymapFileContent, Settings, SettingsFileContent};
 use std::{path::Path, sync::Arc, time::Duration};
 use theme::ThemeRegistry;
 use util::ResultExt;
@@ -44,7 +44,7 @@ where
             fs.load(&path)
                 .await
                 .log_err()
-                .and_then(|data| serde_json::from_str(&data).log_err())
+                .and_then(|data| parse_json_with_comments(&data).log_err())
         } else {
             Some(T::default())
         }
@@ -76,11 +76,14 @@ pub fn settings_from_files(
     })
 }
 
-pub async fn watch_keymap_file(mut file: WatchedJsonFile<KeymapFile>, mut cx: AsyncAppContext) {
+pub async fn watch_keymap_file(
+    mut file: WatchedJsonFile<KeymapFileContent>,
+    mut cx: AsyncAppContext,
+) {
     while let Some(content) = file.0.recv().await {
         cx.update(|cx| {
             cx.clear_bindings();
-            settings::KeymapFile::load_defaults(cx);
+            settings::KeymapFileContent::load_defaults(cx);
             content.add(cx).log_err();
         });
     }

crates/zed/src/zed.rs 🔗

@@ -138,7 +138,7 @@ pub fn init(app_state: &Arc<AppState>, cx: &mut gpui::MutableAppContext) {
 
     workspace::lsp_status::init(cx);
 
-    settings::KeymapFile::load_defaults(cx);
+    settings::KeymapFileContent::load_defaults(cx);
 }
 
 pub fn build_workspace(