Properly handle snippet migration with multiple different snippets in one file and noop phrase

data/fixtures/cursorless-snippets/multipleSnippets.cursorless-snippets

@@ -0,0 +1,149 @@
+{
+  "mobxConstructor": {
+    "definitions": [
+      {
+        "scope": {
+          "langIds": [
+            "typescript",
+            "javascript",
+            "typescriptreact",
+            "javascriptreact"
+          ]
+        },
+        "body": [
+          "constructor($parameters) {",
+          "\tmakeAutoObservable(this);",
+          "}"
+        ]
+      }
+    ],
+    "description": "Constructor using makeAutoObservable",
+    "insertionScopeTypes": [
+      "namedFunction"
+    ]
+  },
+  "constantDeclaration": {
+    "definitions": [
+      {
+        "scope": {
+          "langIds": [
+            "typescript",
+            "javascript",
+            "typescriptreact",
+            "javascriptreact"
+          ]
+        },
+        "body": [
+          "const $name = ${value/^([^;]*);?$/$1/};"
+        ],
+        "variables": {
+          "name": {
+            "formatter": "camelCase"
+          }
+        }
+      }
+    ],
+    "description": "Constant variable declaration",
+    "insertionScopeTypes": [
+      "statement"
+    ],
+    "variables": {
+      "value": {
+        "wrapperScopeType": "statement"
+      }
+    }
+  },
+  "constantDeclarationWithType": {
+    "definitions": [
+      {
+        "scope": {
+          "langIds": [
+            "typescript",
+            "javascript",
+            "typescriptreact",
+            "javascriptreact"
+          ]
+        },
+        "body": [
+          "const $name: $type = ${value/^([^;]*);?$/$1/};"
+        ],
+        "variables": {
+          "name": {
+            "formatter": "camelCase"
+          }
+        }
+      }
+    ],
+    "description": "Constant variable declaration with type",
+    "insertionScopeTypes": [
+      "statement"
+    ],
+    "variables": {
+      "value": {
+        "wrapperScopeType": "statement"
+      }
+    }
+  },
+  "letDeclaration": {
+    "definitions": [
+      {
+        "scope": {
+          "langIds": [
+            "typescript",
+            "javascript",
+            "typescriptreact",
+            "javascriptreact"
+          ]
+        },
+        "body": [
+          "let $name = ${value/^([^;]*);?$/$1/};"
+        ],
+        "variables": {
+          "name": {
+            "formatter": "camelCase"
+          }
+        }
+      }
+    ],
+    "description": "Let variable declaration",
+    "insertionScopeTypes": [
+      "statement"
+    ],
+    "variables": {
+      "value": {
+        "wrapperScopeType": "statement"
+      }
+    }
+  },
+  "letDeclarationWithType": {
+    "definitions": [
+      {
+        "scope": {
+          "langIds": [
+            "typescript",
+            "javascript",
+            "typescriptreact",
+            "javascriptreact"
+          ]
+        },
+        "body": [
+          "let $name: $type = ${value/^([^;]*);?$/$1/};"
+        ],
+        "variables": {
+          "name": {
+            "formatter": "camelCase"
+          }
+        }
+      }
+    ],
+    "description": "Let variable declaration with type",
+    "insertionScopeTypes": [
+      "statement"
+    ],
+    "variables": {
+      "value": {
+        "wrapperScopeType": "statement"
+      }
+    }
+  }
+}

packages/cursorless-vscode/src/migrateSnippets.ts

@@ -21,7 +21,7 @@ interface Result {
   skipped: string[];
 }
 
-interface SpokenForms {
+export interface SpokenForms {
   insertion: Record<string, string>;
   insertionWithPhrase: Record<string, string>;
   wrapper: Record<string, string>;
@@ -64,23 +64,62 @@ async function migrateFile(
   filePath: string,
 ) {
   const fileName = path.basename(filePath, CURSORLESS_SNIPPETS_SUFFIX);
-  const snippetFile = await readLegacyFile(filePath);
+  const legacySnippetFile = await readLegacyFile(filePath);
+
+  const [communitySnippetFile, hasSkippedSnippet] = migrateLegacySnippet(
+    spokenForms,
+    legacySnippetFile,
+  );
+
+  if (communitySnippetFile.snippets.length === 0) {
+    result.skipped.push(fileName);
+    return;
+  }
+
+  const destinationName = await saveSnippetFile(
+    communitySnippetFile,
+    targetDirectory,
+    fileName,
+  );
+
+  if (hasSkippedSnippet) {
+    result.migratedPartially[fileName] = destinationName;
+  } else {
+    result.migrated[fileName] = destinationName;
+  }
+}
+
+export function migrateLegacySnippet(
+  spokenForms: SpokenForms,
+  legacySnippetFile: SnippetMap,
+): [SnippetFile, boolean] {
   const communitySnippetFile: SnippetFile = { snippets: [] };
+  const snippetNames = Object.keys(legacySnippetFile);
+  const useHeader = snippetNames.length === 1;
   let hasSkippedSnippet = false;
 
-  for (const snippetName in snippetFile) {
-    const snippet = snippetFile[snippetName];
+  for (const snippetName of snippetNames) {
+    const snippet = legacySnippetFile[snippetName];
     const phrase =
       spokenForms.insertion[snippetName] ??
       spokenForms.insertionWithPhrase[snippetName];
+    const phrases = phrase ? [phrase] : undefined;
 
-    communitySnippetFile.header = {
-      name: snippetName,
-      description: snippet.description,
-      phrases: phrase ? [phrase] : undefined,
-      variables: parseVariables(spokenForms, snippetName, snippet.variables),
-      insertionScopes: snippet.insertionScopeTypes,
-    };
+    if (useHeader) {
+      communitySnippetFile.header = {
+        name: snippetName,
+        description: snippet.description,
+        phrases: phrases,
+        variables: parseVariables(
+          spokenForms,
+          snippetName,
+          snippet.variables,
+          undefined,
+          true,
+        ),
+        insertionScopes: snippet.insertionScopeTypes,
+      };
+    }
 
     for (const def of snippet.definitions) {
       if (
@@ -91,62 +130,74 @@ async function migrateFile(
         continue;
       }
       communitySnippetFile.snippets.push({
-        body: def.body.map((line) => line.replaceAll("\t", "    ")),
+        name: useHeader ? undefined : snippetName,
+        description: useHeader ? undefined : snippet.description,
+        phrases: useHeader ? undefined : phrases,
+        insertionScopes: useHeader ? undefined : snippet.insertionScopeTypes,
         languages: def.scope?.langIds,
-        variables: parseVariables(spokenForms, snippetName, def.variables),
+        variables: parseVariables(
+          spokenForms,
+          snippetName,
+          useHeader ? undefined : snippet.variables,
+          def.variables,
+          !useHeader,
+        ),
         // SKIP: def.scope?.scopeTypes
         // SKIP: def.scope?.excludeDescendantScopeTypes
+        body: def.body.map((line) => line.replaceAll("\t", "    ")),
       });
     }
   }
 
-  if (communitySnippetFile.snippets.length === 0) {
-    result.skipped.push(fileName);
-    return;
-  }
-
-  let destinationName: string;
-
-  try {
-    destinationName = `${fileName}.snippet`;
-    const destinationPath = path.join(targetDirectory, destinationName);
-    await writeCommunityFile(communitySnippetFile, destinationPath, "wx");
-  } catch (error: any) {
-    if (error.code === "EEXIST") {
-      destinationName = `${fileName}_CONFLICT.snippet`;
-      const destinationPath = path.join(targetDirectory, destinationName);
-      await writeCommunityFile(communitySnippetFile, destinationPath, "w");
-    } else {
-      throw error;
-    }
-  }
-
-  if (hasSkippedSnippet) {
-    result.migratedPartially[fileName] = destinationName;
-  } else {
-    result.migrated[fileName] = destinationName;
-  }
+  return [communitySnippetFile, hasSkippedSnippet];
 }
 
 function parseVariables(
   spokenForms: SpokenForms,
   snippetName: string,
-  variables?: Record<string, SnippetVariableLegacy>,
+  snippetVariables: Record<string, SnippetVariableLegacy> | undefined,
+  defVariables: Record<string, SnippetVariableLegacy> | undefined,
+  addMissingPhrases: boolean,
 ): SnippetVariable[] {
-  return Object.entries(variables ?? {}).map(
-    ([name, variable]): SnippetVariable => {
+  const map: Record<string, SnippetVariable> = {};
+
+  const add = (name: string, variable: SnippetVariableLegacy) => {
+    if (!map[name]) {
       const phrase = spokenForms.wrapper[`${snippetName}.${name}`];
-      return {
+      map[name] = {
         name,
         wrapperPhrases: phrase ? [phrase] : undefined,
-        wrapperScope: variable.wrapperScopeType,
-        insertionFormatters: variable.formatter
-          ? getFormatter(variable.formatter)
-          : undefined,
-        // SKIP: variable.description
       };
-    },
+    }
+    if (variable.wrapperScopeType) {
+      map[name].wrapperScope = variable.wrapperScopeType;
+    }
+    if (variable.formatter) {
+      map[name].insertionFormatters = getFormatter(variable.formatter);
+    }
+    // SKIP: variable.description
+  };
+
+  Object.entries(snippetVariables ?? {}).forEach(([name, variable]) =>
+    add(name, variable),
+  );
+  Object.entries(defVariables ?? {}).forEach(([name, variable]) =>
+    add(name, variable),
   );
+
+  if (addMissingPhrases) {
+    for (const key in spokenForms.wrapper) {
+      const [snipName, variableName] = key.split(".");
+      if (snipName === snippetName && !map[variableName]) {
+        map[variableName] = {
+          name: variableName,
+          wrapperPhrases: [spokenForms.wrapper[key]],
+        };
+      }
+    }
+  }
+
+  return Object.values(map);
 }
 
 // Convert Cursorless formatters to Talon community formatters
@@ -165,6 +216,30 @@ function getFormatter(formatter: string): string[] {
   }
 }
 
+async function saveSnippetFile(
+  communitySnippetFile: SnippetFile,
+  targetDirectory: string,
+  fileName: string,
+) {
+  let destinationName: string;
+
+  try {
+    destinationName = `${fileName}.snippet`;
+    const destinationPath = path.join(targetDirectory, destinationName);
+    await writeCommunityFile(communitySnippetFile, destinationPath, "wx");
+  } catch (error: any) {
+    if (error.code === "EEXIST") {
+      destinationName = `${fileName}_CONFLICT.snippet`;
+      const destinationPath = path.join(targetDirectory, destinationName);
+      await writeCommunityFile(communitySnippetFile, destinationPath, "w");
+    } else {
+      throw error;
+    }
+  }
+
+  return destinationName;
+}
+
 async function openResultDocument(
   result: Result,
   sourceDirectory: string,