Support multiple snippets locations per extension (#45829)

Finn Evers created 2 weeks ago

This allows extensions to add more than one snippet file whilst keeping
it backwards compatible.

Release Notes:

- Added support for specifying multiple snippets paths in extensions.

Change summary

crates/extension/src/extension_builder.rs   | 10 ++++--
crates/extension/src/extension_manifest.rs  | 26 ++++++++++++++++
crates/extension_cli/src/main.rs            | 34 ++++++++++++----------
crates/extension_host/src/extension_host.rs | 10 ++++--
4 files changed, 57 insertions(+), 23 deletions(-)

Detailed changes

crates/extension/src/extension_builder.rs

@@ -717,7 +717,7 @@ mod tests {
     use indoc::indoc;
 
     use crate::{
-        ExtensionManifest,
+        ExtensionManifest, ExtensionSnippets,
         extension_builder::{file_newer_than_deps, populate_defaults},
     };
 
@@ -791,7 +791,9 @@ mod tests {
 
         assert_eq!(
             manifest.snippets,
-            Some(PathBuf::from_str("./snippets/snippets.json").unwrap())
+            Some(ExtensionSnippets::Single(
+                PathBuf::from_str("./snippets/snippets.json").unwrap()
+            ))
         )
     }
 
@@ -826,7 +828,9 @@ mod tests {
 
         assert_eq!(
             manifest.snippets,
-            Some(PathBuf::from_str("snippets.json").unwrap())
+            Some(ExtensionSnippets::Single(
+                PathBuf::from_str("snippets.json").unwrap()
+            ))
         )
     }
 }

crates/extension/src/extension_manifest.rs

@@ -53,6 +53,30 @@ impl SchemaVersion {
     }
 }
 
+// TODO: We should change this to just always be a Vec<PathBuf> once we bump the
+// extension.toml schema version to 2
+#[derive(Debug, PartialEq, Eq, Clone, Serialize, Deserialize)]
+#[serde(untagged)]
+pub enum ExtensionSnippets {
+    Single(PathBuf),
+    Multiple(Vec<PathBuf>),
+}
+
+impl ExtensionSnippets {
+    pub fn paths(&self) -> impl Iterator<Item = &PathBuf> {
+        match self {
+            ExtensionSnippets::Single(path) => std::slice::from_ref(path).iter(),
+            ExtensionSnippets::Multiple(paths) => paths.iter(),
+        }
+    }
+}
+
+impl From<&str> for ExtensionSnippets {
+    fn from(value: &str) -> Self {
+        ExtensionSnippets::Single(value.into())
+    }
+}
+
 #[derive(Debug, PartialEq, Eq, Clone, Serialize, Deserialize)]
 pub struct ExtensionManifest {
     pub id: Arc<str>,
@@ -86,7 +110,7 @@ pub struct ExtensionManifest {
     #[serde(default)]
     pub slash_commands: BTreeMap<Arc<str>, SlashCommandManifestEntry>,
     #[serde(default)]
-    pub snippets: Option<PathBuf>,
+    pub snippets: Option<ExtensionSnippets>,
     #[serde(default)]
     pub capabilities: Vec<ExtensionCapability>,
     #[serde(default, skip_serializing_if = "BTreeMap::is_empty")]

crates/extension_cli/src/main.rs

@@ -306,22 +306,26 @@ async fn copy_extension_resources(
         }
     }
 
-    if let Some(snippets_path) = manifest.snippets.as_ref() {
-        let parent = snippets_path.parent();
-        if let Some(parent) = parent.filter(|p| p.components().next().is_some()) {
-            fs::create_dir_all(output_dir.join(parent))?;
+    if let Some(snippets) = manifest.snippets.as_ref() {
+        for snippets_path in snippets.paths() {
+            let parent = snippets_path.parent();
+            if let Some(parent) = parent.filter(|p| p.components().next().is_some()) {
+                fs::create_dir_all(output_dir.join(parent))?;
+            }
+            copy_recursive(
+                fs.as_ref(),
+                &extension_path.join(&snippets_path),
+                &output_dir.join(&snippets_path),
+                CopyOptions {
+                    overwrite: true,
+                    ignore_if_exists: false,
+                },
+            )
+            .await
+            .with_context(|| {
+                format!("failed to copy snippets from '{}'", snippets_path.display())
+            })?;
         }
-        copy_recursive(
-            fs.as_ref(),
-            &extension_path.join(&snippets_path),
-            &output_dir.join(&snippets_path),
-            CopyOptions {
-                overwrite: true,
-                ignore_if_exists: false,
-            },
-        )
-        .await
-        .with_context(|| format!("failed to copy snippets from '{}'", snippets_path.display()))?;
     }
 
     Ok(())

crates/extension_host/src/extension_host.rs

@@ -1252,10 +1252,12 @@ impl ExtensionStore {
                     (path, icons_root_path)
                 },
             ));
-            snippets_to_add.extend(extension.manifest.snippets.iter().map(|snippets_path| {
-                let mut path = self.installed_dir.clone();
-                path.extend([Path::new(extension_id.as_ref()), snippets_path.as_path()]);
-                path
+            snippets_to_add.extend(extension.manifest.snippets.iter().flat_map(|snippets| {
+                snippets.paths().map(|snippets_path| {
+                    let mut path = self.installed_dir.clone();
+                    path.extend([Path::new(extension_id.as_ref()), snippets_path.as_path()]);
+                    path
+                })
             }));
         }