Merge pull request #865 from posit-dev/task/refactor-config

lionel- · web-flow · commit 8336b329c7c6 · 2025-07-22T16:59:27.000+02:00
Simplify LSP settings
diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md
@@ -22,3 +22,5 @@ judgement to produce the clearest code organization.
 
  Keep the main logic as unnested as possible. Favour Rust's `let ... else`
  syntax to return early or continue a loop in the `else` clause, over `if let`.
+
+Always prefer importing with `use` instead of qualifying with `::`, unless specifically requested in these instructions or by the user, or you see existing `::` usages in the file you're editing.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/crates/ark/Cargo.toml b/crates/ark/Cargo.toml
@@ -53,7 +53,6 @@ url = "2.4.1"
 walkdir = "2"
 yaml-rust = "0.4.5"
 winsafe = { version = "0.0.19", features = ["kernel"] }
-struct-field-names-as-array = "0.3.0"
 strum = "0.26.2"
 strum_macros = "0.26.2"
 futures = "0.3.30"
diff --git a/crates/ark/src/lsp/config.rs b/crates/ark/src/lsp/config.rs
@@ -1,10 +1,77 @@
 use serde::Deserialize;
 use serde::Serialize;
-use struct_field_names_as_array::FieldNamesAsArray;
+use serde_json::Value;
 
-use crate::lsp;
 use crate::lsp::diagnostics::DiagnosticsConfig;
 
+pub struct Setting<T> {
+    pub key: &'static str,
+    pub set: fn(&mut T, Value),
+}
+
+/// List of LSP settings for which clients can send `didChangeConfiguration`
+/// notifications. We register our interest in watching over these settings in
+/// our `initialized` handler. The `set` methods convert from a json `Value` to
+/// the expected type, using a default value if the conversion fails.
+///
+/// This array is for global settings. If the setting should only affect a given
+/// document URI, add it to `DOCUMENT_SETTINGS` instead.
+pub static GLOBAL_SETTINGS: &[Setting<LspConfig>] = &[
+    Setting {
+        key: "positron.r.diagnostics.enable",
+        set: |cfg, v| {
+            cfg.diagnostics.enable = v
+                .as_bool()
+                .unwrap_or_else(|| DiagnosticsConfig::default().enable)
+        },
+    },
+    Setting {
+        key: "positron.r.symbols.includeAssignmentsInBlocks",
+        set: |cfg, v| {
+            cfg.symbols.include_assignments_in_blocks = v
+                .as_bool()
+                .unwrap_or_else(|| SymbolsConfig::default().include_assignments_in_blocks)
+        },
+    },
+];
+
+/// These document settings are updated on a URI basis. Each document has its
+/// own value of the setting.
+pub static DOCUMENT_SETTINGS: &[Setting<DocumentConfig>] = &[
+    Setting {
+        key: "editor.insertSpaces",
+        set: |cfg, v| {
+            let default_style = IndentationConfig::default().indent_style;
+            cfg.indent.indent_style = if v
+                .as_bool()
+                .unwrap_or_else(|| default_style == IndentStyle::Space)
+            {
+                IndentStyle::Space
+            } else {
+                IndentStyle::Tab
+            }
+        },
+    },
+    Setting {
+        key: "editor.indentSize",
+        set: |cfg, v| {
+            cfg.indent.indent_size = v
+                .as_u64()
+                .map(|n| n as usize)
+                .unwrap_or_else(|| IndentationConfig::default().indent_size)
+        },
+    },
+    Setting {
+        key: "editor.tabSize",
+        set: |cfg, v| {
+            cfg.indent.tab_width = v
+                .as_u64()
+                .map(|n| n as usize)
+                .unwrap_or_else(|| IndentationConfig::default().tab_width)
+        },
+    },
+];
+
 /// Configuration of the LSP
 #[derive(Clone, Default, Debug)]
 pub(crate) struct LspConfig {
@@ -39,40 +106,12 @@ pub struct IndentationConfig {
     pub tab_width: usize,
 }
 
-#[derive(Serialize, Deserialize, Clone, Debug)]
+#[derive(PartialEq, Serialize, Deserialize, Clone, Debug)]
 pub enum IndentStyle {
     Tab,
     Space,
 }
 
-/// VS Code representation of a document configuration
-#[derive(Serialize, Deserialize, FieldNamesAsArray, Clone, Debug)]
-pub(crate) struct VscDocumentConfig {
-    // DEV NOTE: Update `section_from_key()` method after adding a field
-    pub insert_spaces: bool,
-    pub indent_size: VscIndentSize,
-    pub tab_size: usize,
-}
-
-#[derive(Serialize, Deserialize, FieldNamesAsArray, Clone, Debug)]
-pub(crate) struct VscDiagnosticsConfig {
-    // DEV NOTE: Update `section_from_key()` method after adding a field
-    pub enable: bool,
-}
-
-#[derive(Serialize, Deserialize, FieldNamesAsArray, Clone, Debug)]
-pub(crate) struct VscSymbolsConfig {
-    // DEV NOTE: Update `section_from_key()` method after adding a field
-    pub include_assignments_in_blocks: bool,
-}
-
-#[derive(Serialize, Deserialize, Clone, Debug)]
-#[serde(untagged)]
-pub(crate) enum VscIndentSize {
-    Alias(String),
-    Size(usize),
-}
-
 impl Default for SymbolsConfig {
     fn default() -> Self {
         Self {
@@ -91,79 +130,6 @@ impl Default for IndentationConfig {
     }
 }
 
-impl VscDocumentConfig {
-    pub(crate) fn section_from_key(key: &str) -> &str {
-        match key {
-            "insert_spaces" => "editor.insertSpaces",
-            "indent_size" => "editor.indentSize",
-            "tab_size" => "editor.tabSize",
-            _ => "unknown", // To be caught via downstream errors
-        }
-    }
-}
-
-/// Convert from VS Code representation of a document config to our own
-/// representation. Currently one-to-one.
-impl From<VscDocumentConfig> for DocumentConfig {
-    fn from(x: VscDocumentConfig) -> Self {
-        let indent_style = indent_style_from_lsp(x.insert_spaces);
-
-        let indent_size = match x.indent_size {
-            VscIndentSize::Size(size) => size,
-            VscIndentSize::Alias(var) => {
-                if var == "tabSize" {
-                    x.tab_size
-                } else {
-                    lsp::log_warn!("Unknown indent alias {var}, using default");
-                    2
-                }
-            },
-        };
-
-        Self {
-            indent: IndentationConfig {
-                indent_style,
-                indent_size,
-                tab_width: x.tab_size,
-            },
-        }
-    }
-}
-
-impl VscDiagnosticsConfig {
-    pub(crate) fn section_from_key(key: &str) -> &str {
-        match key {
-            "enable" => "positron.r.diagnostics.enable",
-            _ => "unknown", // To be caught via downstream errors
-        }
-    }
-}
-
-impl From<VscDiagnosticsConfig> for DiagnosticsConfig {
-    fn from(value: VscDiagnosticsConfig) -> Self {
-        Self {
-            enable: value.enable,
-        }
-    }
-}
-
-impl VscSymbolsConfig {
-    pub(crate) fn section_from_key(key: &str) -> &str {
-        match key {
-            "include_assignments_in_blocks" => "positron.r.symbols.includeAssignmentsInBlocks",
-            _ => "unknown", // To be caught via downstream errors
-        }
-    }
-}
-
-impl From<VscSymbolsConfig> for SymbolsConfig {
-    fn from(value: VscSymbolsConfig) -> Self {
-        Self {
-            include_assignments_in_blocks: value.include_assignments_in_blocks,
-        }
-    }
-}
-
 pub(crate) fn indent_style_from_lsp(insert_spaces: bool) -> IndentStyle {
     if insert_spaces {
         IndentStyle::Space
diff --git a/crates/ark/src/lsp/handlers.rs b/crates/ark/src/lsp/handlers.rs
@@ -9,7 +9,6 @@ use anyhow::anyhow;
 use serde_json::Value;
 use stdext::unwrap;
 use stdext::unwrap::IntoResult;
-use struct_field_names_as_array::FieldNamesAsArray;
 use tower_lsp::lsp_types::CodeActionParams;
 use tower_lsp::lsp_types::CodeActionResponse;
 use tower_lsp::lsp_types::CompletionItem;
@@ -46,9 +45,6 @@ use crate::lsp;
 use crate::lsp::code_action::code_actions;
 use crate::lsp::completions::provide_completions;
 use crate::lsp::completions::resolve_completion;
-use crate::lsp::config::VscDiagnosticsConfig;
-use crate::lsp::config::VscDocumentConfig;
-use crate::lsp::config::VscSymbolsConfig;
 use crate::lsp::definitions::goto_definition;
 use crate::lsp::document_context::DocumentContext;
 use crate::lsp::encoding::convert_lsp_range_to_tree_sitter_range;
@@ -106,22 +102,21 @@ pub(crate) async fn handle_initialized(
         // Note that some settings, such as editor indentation properties, may be
         // changed by extensions or by the user without changing the actual
         // underlying setting. Unfortunately we don't receive updates in that case.
-        let mut config_document_regs = collect_regs(
-            VscDocumentConfig::FIELD_NAMES_AS_ARRAY.to_vec(),
-            VscDocumentConfig::section_from_key,
-        );
-        let mut config_symbols_regs: Vec<Registration> = collect_regs(
-            VscSymbolsConfig::FIELD_NAMES_AS_ARRAY.to_vec(),
-            VscSymbolsConfig::section_from_key,
-        );
-        let mut config_diagnostics_regs: Vec<Registration> = collect_regs(
-            VscDiagnosticsConfig::FIELD_NAMES_AS_ARRAY.to_vec(),
-            VscDiagnosticsConfig::section_from_key,
-        );
-
-        regs.append(&mut config_document_regs);
-        regs.append(&mut config_symbols_regs);
-        regs.append(&mut config_diagnostics_regs);
+
+        for setting in crate::lsp::config::GLOBAL_SETTINGS {
+            regs.push(Registration {
+                id: uuid::Uuid::new_v4().to_string(),
+                method: String::from("workspace/didChangeConfiguration"),
+                register_options: Some(serde_json::json!({ "section": setting.key })),
+            });
+        }
+        for setting in crate::lsp::config::DOCUMENT_SETTINGS {
+            regs.push(Registration {
+                id: uuid::Uuid::new_v4().to_string(),
+                method: String::from("workspace/didChangeConfiguration"),
+                register_options: Some(serde_json::json!({ "section": setting.key })),
+            });
+        }
     }
 
     client
@@ -131,17 +126,6 @@ pub(crate) async fn handle_initialized(
     Ok(())
 }
 
-fn collect_regs(fields: Vec<&str>, into_section: impl Fn(&str) -> &str) -> Vec<Registration> {
-    fields
-        .into_iter()
-        .map(|field| Registration {
-            id: uuid::Uuid::new_v4().to_string(),
-            method: String::from("workspace/didChangeConfiguration"),
-            register_options: Some(serde_json::json!({ "section": into_section(field) })),
-        })
-        .collect()
-}
-
 #[tracing::instrument(level = "info", skip_all)]
 pub(crate) fn handle_symbol(
     params: WorkspaceSymbolParams,
diff --git a/crates/ark/src/lsp/state_handlers.rs b/crates/ark/src/lsp/state_handlers.rs
