From 2b4169b5db1ffbdcce1e5fc9bb4be4f2d71f739d Mon Sep 17 00:00:00 2001 From: Mathis Date: Wed, 16 Sep 2026 15:58:12 +0000 Subject: [PATCH] snippet_provider: Fix multiple snippets for same language (#58659) This PR fixes the snippets issue when a user loads multiple snippets for the same language. It also adds tests for the snippets register function. **Note** I've made the choice to differentiate snippets based on their body. Thus, two extensions can provide the same snippet with the same prefix but with different bodies. WDYT ? Self-Review Checklist: - [x] I've reviewed my own diff for quality, security, and reliability - [x] Unsafe blocks (if any) have justifying comments - [x] The content is consistent with the [UI/UX checklist](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist) - [x] Tests cover the new/changed behavior - [x] Performance impact has been considered and is acceptable Closes #51014 Closes #52182 Release Notes: - Fixed different snippet extensions for the same language cancel each other --------- Co-authored-by: MrSubidubi --- crates/snippet_provider/src/registry.rs | 191 +++++++++++++++++++++++- 1 file changed, 189 insertions(+), 2 deletions(-) diff --git a/crates/snippet_provider/src/registry.rs b/crates/snippet_provider/src/registry.rs index ae721ee632..17c6096295 100644 --- a/crates/snippet_provider/src/registry.rs +++ b/crates/snippet_provider/src/registry.rs @@ -43,10 +43,14 @@ impl SnippetRegistry { let kind = file_path .file_stem() .and_then(|stem| stem.to_str().and_then(file_stem_to_key)); - let snippets = crate::file_to_snippets(snippets_in_file, file_path); + let new_snippets = + crate::file_to_snippets(snippets_in_file, file_path).filter_map(Result::log_err); + self.snippets .write() - .insert(kind, snippets.filter_map(Result::log_err).collect()); + .entry(kind) + .or_default() + .extend(new_snippets); Ok(()) } @@ -55,3 +59,186 @@ impl SnippetRegistry { self.snippets.read().get(kind).cloned().unwrap_or_default() } } + +#[cfg(test)] +mod tests { + use super::*; + use std::path::Path; + + #[test] + fn test_register_snippets_single_language() { + let registry = SnippetRegistry::new(); + registry + .register_snippets( + Path::new("rust.json"), + r#"{"Hello World": {"prefix": "hello", "body": "Hello, ${1:World}!"}}"#, + ) + .unwrap(); + + let snippets = registry.get_snippets(&Some("rust".to_owned())); + assert_eq!(snippets.len(), 1); + assert_eq!(snippets[0].name, "Hello World"); + assert_eq!(snippets[0].prefix, vec!["hello".to_owned()]); + } + + #[test] + fn test_register_snippets_two_extensions_same_language() { + let registry = SnippetRegistry::new(); + registry + .register_snippets( + Path::new("ruby.json"), + r#"{"Snippet One": {"prefix": "one", "body": "snippet_one"}}"#, + ) + .unwrap(); + registry + .register_snippets( + Path::new("ruby.json"), + r#"{"Snippet Two": {"prefix": "two", "body": "snippet_two"}}"#, + ) + .unwrap(); + + let snippets = registry.get_snippets(&Some("ruby".to_owned())); + assert_eq!( + snippets.len(), + 2, + "Snippets from both extensions should be present" + ); + let names: Vec<&str> = snippets.iter().map(|s| s.name.as_str()).collect(); + assert!(names.contains(&"Snippet One")); + assert!(names.contains(&"Snippet Two")); + } + + #[test] + fn test_register_snippets_same_first_prefix_preserved() -> Result<()> { + let registry = SnippetRegistry::new(); + registry.register_snippets( + Path::new("ruby.json"), + r#"{"For Loop": {"prefix": "for", "body": "for ${1:i} in ${2:iter} do\n$0\nend"}}"#, + )?; + registry.register_snippets( + Path::new("ruby.json"), + r#"{"Different Name": {"prefix": "for", "body": "${2:iter}.each do |${1:item}|\n$0\nend"}}"#, + )?; + + let snippets = registry.get_snippets(&Some("ruby".to_owned())); + assert_eq!( + snippets.len(), + 2, + "Snippets sharing the same first prefix should both be preserved" + ); + assert_eq!(snippets[0].name, "For Loop"); + assert_eq!(snippets[0].prefix, vec!["for".to_owned()]); + assert_eq!(snippets[1].name, "Different Name"); + assert_eq!(snippets[1].prefix, vec!["for".to_owned()]); + Ok(()) + } + + #[test] + fn test_register_snippets_same_first_prefix_with_extra_prefixes_preserved() -> Result<()> { + let registry = SnippetRegistry::new(); + registry.register_snippets( + Path::new("ruby.json"), + r#"{"For Loop": {"prefix": "for", "body": "for ${1:i} in ${2:iter} do\n$0\nend"}}"#, + )?; + registry.register_snippets( + Path::new("ruby.json"), + r#"{"For Loop Alt": {"prefix": ["for", "floop"], "body": "${2:iter}.each do |${1:item}|\n$0\nend"}}"#, + )?; + + let snippets = registry.get_snippets(&Some("ruby".to_owned())); + assert_eq!( + snippets.len(), + 2, + "Snippets sharing the same first prefix should be preserved along with their extra prefixes" + ); + assert_eq!(snippets[0].name, "For Loop"); + assert_eq!(snippets[0].prefix, vec!["for".to_owned()]); + assert_eq!(snippets[1].name, "For Loop Alt"); + assert_eq!( + snippets[1].prefix, + vec!["for".to_owned(), "floop".to_owned()] + ); + Ok(()) + } + + #[test] + fn test_register_snippets_different_first_prefixes_not_deduplicated() { + let registry = SnippetRegistry::new(); + registry + .register_snippets( + Path::new("ruby.json"), + r#"{"For Loop": {"prefix": "for", "body": "for ${1:i} in ${2:iter} do\n$0\nend"}}"#, + ) + .unwrap(); + registry + .register_snippets( + Path::new("ruby.json"), + r#"{"While Loop": {"prefix": "while", "body": "while ${1:cond}\n$0\nend"}}"#, + ) + .unwrap(); + + let snippets = registry.get_snippets(&Some("ruby".to_owned())); + assert_eq!( + snippets.len(), + 2, + "Snippets with different first prefixes should not be deduplicated" + ); + } + + #[test] + fn test_register_global_snippets() { + let registry = SnippetRegistry::new(); + registry + .register_snippets( + Path::new("snippets.json"), + r#"{"Global Snippet": {"prefix": "global", "body": "a global snippet"}}"#, + ) + .unwrap(); + + let global_snippets = registry.get_snippets(&None); + assert_eq!(global_snippets.len(), 1); + assert_eq!(global_snippets[0].name, "Global Snippet"); + + let miskeyed = registry.get_snippets(&Some("snippets".to_owned())); + assert!( + miskeyed.is_empty(), + "Should not be stored under the language key 'snippets'" + ); + } + + #[test] + fn test_register_snippets_different_languages_are_isolated() { + let registry = SnippetRegistry::new(); + registry + .register_snippets( + Path::new("rust.json"), + r#"{"Rust Snippet": {"prefix": "rsnip", "body": "fn ${1:name}() {}"}}"#, + ) + .unwrap(); + registry + .register_snippets( + Path::new("python.json"), + r#"{"Python Snippet": {"prefix": "pysnip", "body": "def ${1:name}():"}}"#, + ) + .unwrap(); + + let rust_snippets = registry.get_snippets(&Some("rust".to_owned())); + assert_eq!(rust_snippets.len(), 1); + assert_eq!(rust_snippets[0].name, "Rust Snippet"); + + let python_snippets = registry.get_snippets(&Some("python".to_owned())); + assert_eq!(python_snippets.len(), 1); + assert_eq!(python_snippets[0].name, "Python Snippet"); + } + + #[test] + fn test_get_snippets_unknown_language_returns_empty() { + let registry = SnippetRegistry::new(); + assert!( + registry + .get_snippets(&Some("nonexistent".to_owned())) + .is_empty() + ); + assert!(registry.get_snippets(&None).is_empty()); + } +}