Exclude comment sections from workspace symbols by default

.github/copilot-instructions.md

@@ -19,3 +19,6 @@ _below_ the calling function. A general goal is to be able to read linearly from
 top to bottom with the relevant context and main logic first. The code should be
 organised like a call stack. Of course that's not always possible, use best
 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`.

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"

crates/ark/src/lsp/config.rs

@@ -1,14 +1,96 @@
 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 {
+    pub key: &'static str,
+    pub set: fn(&mut LspConfig, 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.
+pub static SETTINGS: &[Setting] = &[
+    Setting {
+        key: "editor.insertSpaces",
+        set: |cfg, v| {
+            let default_style = IndentationConfig::default().indent_style;
+            cfg.document.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.document.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.document.indent.tab_width = v
+                .as_u64()
+                .map(|n| n as usize)
+                .unwrap_or_else(|| IndentationConfig::default().tab_width)
+        },
+    },
+    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)
+        },
+    },
+    Setting {
+        key: "positron.r.workspaceSymbols.includeCommentSections",
+        set: |cfg, v| {
+            cfg.workspace_symbols.include_comment_sections = v
+                .as_bool()
+                .unwrap_or_else(|| WorkspaceSymbolsConfig::default().include_comment_sections)
+        },
+    },
+];
+
 /// Configuration of the LSP
-#[derive(Clone, Debug)]
+#[derive(Clone, Default, Debug)]
 pub(crate) struct LspConfig {
     pub(crate) diagnostics: DiagnosticsConfig,
+    pub(crate) symbols: SymbolsConfig,
+    pub(crate) workspace_symbols: WorkspaceSymbolsConfig,
+    pub(crate) document: DocumentConfig,
+}
+
+#[derive(Serialize, Deserialize, Clone, Debug)]
+pub struct SymbolsConfig {
+    /// Whether to emit assignments in `{` bloks as document symbols.
+    pub include_assignments_in_blocks: bool,
+}
+
+#[derive(Serialize, Deserialize, Clone, Debug)]
+pub struct WorkspaceSymbolsConfig {
+    /// Whether to include sections like `# My section ---` in workspace symbols.
+    pub include_comment_sections: bool,
 }
 
 /// Configuration of a document.
@@ -32,104 +114,62 @@ 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)]
+#[derive(Serialize, Deserialize, 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)]
+#[derive(Serialize, Deserialize, Clone, Debug)]
 pub(crate) struct VscDiagnosticsConfig {
     // DEV NOTE: Update `section_from_key()` method after adding a field
     pub enable: bool,
 }
 
+#[derive(Serialize, Deserialize, 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 LspConfig {
+impl Default for SymbolsConfig {
     fn default() -> Self {
         Self {
-            diagnostics: Default::default(),
+            include_assignments_in_blocks: false,
         }
     }
 }
 
-impl Default for IndentationConfig {
+impl Default for WorkspaceSymbolsConfig {
     fn default() -> Self {
         Self {
-            indent_style: IndentStyle::Space,
-            indent_size: 2,
-            tab_width: 2,
+            include_comment_sections: false,
         }
     }
 }
 
-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 {
+impl Default for IndentationConfig {
+    fn default() -> Self {
         Self {
-            enable: value.enable,
+            indent_style: IndentStyle::Space,
+            indent_size: 2,
+            tab_width: 2,
         }
     }
 }

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,8 +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::definitions::goto_definition;
 use crate::lsp::document_context::DocumentContext;
 use crate::lsp::encoding::convert_lsp_range_to_tree_sitter_range;
@@ -105,17 +102,16 @@ 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_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_diagnostics_regs);
+
+        use crate::lsp::config::SETTINGS;
+
+        for setting in 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
@@ -125,22 +121,12 @@ 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,
+    state: &WorldState,
 ) -> anyhow::Result<Option<Vec<SymbolInformation>>> {
-    symbols::symbols(&params)
+    symbols::symbols(&params, state)
         .map(|res| Some(res))
         .or_else(|err| {
             // Missing doc: Why are we not propagating errors to the frontend?

crates/ark/src/lsp/indexer.rs

@@ -121,10 +121,17 @@ fn insert(path: &Path, entry: IndexEntry) -> anyhow::Result<()> {
 
     let index = index.entry(path.to_string()).or_default();
 
-    // Retain the first occurrence in the index. In the future we'll track every occurrences and
-    // their scopes but for now we only track the first definition of an object (in a way, its
+    // We generally retain only the first occurrence in the index. In the
+    // future we'll track every occurrences and their scopes but for now we
+    // only track the first definition of an object (in a way, its
     // declaration).
-    if !index.contains_key(&entry.key) {
+    if let Some(existing_entry) = index.get(&entry.key) {
+        // Give priority to non-section entries.
+        if matches!(existing_entry.data, IndexEntryData::Section { .. }) {
+            index.insert(entry.key.clone(), entry);
+        }
+        // Else, ignore.
+    } else {
         index.insert(entry.key.clone(), entry);
     }
 

crates/ark/src/lsp/main_loop.rs

@@ -281,7 +281,7 @@ impl GlobalState {
                             respond(tx, || state_handlers::initialize(params, &mut self.lsp_state, &mut self.world), LspResponse::Initialize)?;
                         },
                         LspRequest::WorkspaceSymbol(params) => {
-                            respond(tx, || handlers::handle_symbol(params), LspResponse::WorkspaceSymbol)?;
+                            respond(tx, || handlers::handle_symbol(params, &self.world), LspResponse::WorkspaceSymbol)?;
                         },
                         LspRequest::DocumentSymbol(params) => {
                             respond(tx, || handlers::handle_document_symbol(params, &self.world), LspResponse::DocumentSymbol)?;