diff --git a/cursorless-talon/src/check_community_repo.py b/cursorless-talon/src/check_community_repo.py index 79d0a6a677..d855546b8b 100644 --- a/cursorless-talon/src/check_community_repo.py +++ b/cursorless-talon/src/check_community_repo.py @@ -8,6 +8,7 @@ ] required_actions = [ + "code.language", "user.homophones_get", "user.insert_snippet_by_name", "user.reformat_text", diff --git a/cursorless-talon/src/snippets/snippet_types.py b/cursorless-talon/src/snippets/snippet_types.py index f9c2af486a..774c156867 100644 --- a/cursorless-talon/src/snippets/snippet_types.py +++ b/cursorless-talon/src/snippets/snippet_types.py @@ -62,6 +62,7 @@ def create( @dataclass class ListInsertionSnippet: type = "list" + fallbackLanguage: str | None substitutions: dict[str, str] | None snippets: list[CustomInsertionSnippet] @@ -98,6 +99,7 @@ def create(snippet: CommunityWrapperSnippet): @dataclass class ListWrapperSnippet: type = "list" + fallbackLanguage: str | None snippets: list[CustomWrapperSnippet] diff --git a/cursorless-talon/src/snippets/snippets.py b/cursorless-talon/src/snippets/snippets.py index 0bd0982608..5facad8463 100644 --- a/cursorless-talon/src/snippets/snippets.py +++ b/cursorless-talon/src/snippets/snippets.py @@ -100,7 +100,7 @@ class UserActions: def insert_snippet_by_name( name: str, # pyright: ignore [reportGeneralTypeIssues] # Don't add optional: we need to match the type in community - substitutions: dict[str, str] = None, + substitutions: dict[str, str] = None, # type: ignore ): action = InsertSnippetAction( get_list_insertion_snippet(name, substitutions), diff --git a/cursorless-talon/src/snippets/snippets_get.py b/cursorless-talon/src/snippets/snippets_get.py index 8aba597dee..48c76c930c 100644 --- a/cursorless-talon/src/snippets/snippets_get.py +++ b/cursorless-talon/src/snippets/snippets_get.py @@ -30,6 +30,7 @@ def get_list_insertion_snippet( raise return ListInsertionSnippet( + get_fallback_language(), substitutions, [CustomInsertionSnippet.create(s) for s in snippets], ) @@ -49,5 +50,13 @@ def get_list_wrapper_snippet(name: str) -> ListWrapperSnippet | CustomWrapperSni raise return ListWrapperSnippet( + get_fallback_language(), [CustomWrapperSnippet.create(s) for s in snippets], ) + + +def get_fallback_language(): + language = actions.code.language() + if language and isinstance(language, str): + return language + return None diff --git a/data/fixtures/recorded/actions/snippets/ifWrapAir.yml b/data/fixtures/recorded/actions/snippets/ifWrapAir.yml new file mode 100644 index 0000000000..bf5637118f --- /dev/null +++ b/data/fixtures/recorded/actions/snippets/ifWrapAir.yml @@ -0,0 +1,42 @@ +languageId: typescript +command: + version: 7 + spokenForm: if wrap air + action: + name: wrapWithSnippet + snippetDescription: + type: list + snippets: + - type: custom + body: "if ($1) {\n\t$0\n}" + variableName: "0" + languages: [c, cpp, csharp, java, javascript, typescript, javascriptreact, typescriptreact] + target: + type: primitive + mark: {type: decoratedSymbol, symbolColor: default, character: a} + usePrePhraseSnapshot: true +spokenFormError: list wrap with snippet +initialState: + documentContents: aaa + selections: + - anchor: {line: 0, character: 0} + active: {line: 0, character: 0} + marks: + default.a: + start: {line: 0, character: 0} + end: {line: 0, character: 3} +finalState: + documentContents: |- + if () { + aaa + } + selections: + - anchor: {line: 0, character: 4} + active: {line: 0, character: 4} + thatMark: + - type: UntypedTarget + contentRange: + start: {line: 0, character: 0} + end: {line: 2, character: 1} + isReversed: false + hasExplicitRange: true diff --git a/data/fixtures/recorded/actions/snippets/ifWrapAir2.yml b/data/fixtures/recorded/actions/snippets/ifWrapAir2.yml new file mode 100644 index 0000000000..620789a591 --- /dev/null +++ b/data/fixtures/recorded/actions/snippets/ifWrapAir2.yml @@ -0,0 +1,43 @@ +languageId: plaintext +command: + version: 7 + spokenForm: if wrap air + action: + name: wrapWithSnippet + snippetDescription: + type: list + fallbackLanguage: typescript + snippets: + - type: custom + body: "if ($1) {\n\t$0\n}" + variableName: "0" + languages: [c, cpp, csharp, java, javascript, typescript, javascriptreact, typescriptreact] + target: + type: primitive + mark: {type: decoratedSymbol, symbolColor: default, character: a} + usePrePhraseSnapshot: true +spokenFormError: list wrap with snippet +initialState: + documentContents: aaa + selections: + - anchor: {line: 0, character: 0} + active: {line: 0, character: 0} + marks: + default.a: + start: {line: 0, character: 0} + end: {line: 0, character: 3} +finalState: + documentContents: |- + if () { + aaa + } + selections: + - anchor: {line: 0, character: 4} + active: {line: 0, character: 4} + thatMark: + - type: UntypedTarget + contentRange: + start: {line: 0, character: 0} + end: {line: 2, character: 1} + isReversed: false + hasExplicitRange: true diff --git a/data/fixtures/recorded/actions/snippets/snipIf2.yml b/data/fixtures/recorded/actions/snippets/snipIf2.yml new file mode 100644 index 0000000000..e368aedba0 --- /dev/null +++ b/data/fixtures/recorded/actions/snippets/snipIf2.yml @@ -0,0 +1,49 @@ +languageId: plaintext +command: + version: 7 + spokenForm: snip if + action: + name: insertSnippet + snippetDescription: + type: list + fallbackLanguage: typescript + snippets: + - type: custom + body: "if ($1) {\n\t$0\n}" + scopeTypes: + - {type: statement} + languages: [c, cpp, csharp, java, javascript, typescript, javascriptreact, typescriptreact] + - type: custom + body: "if $1:\n\t$0" + scopeTypes: + - {type: statement} + languages: [python] + - type: custom + body: "if $1 then\n\t$0\nend" + scopeTypes: + - {type: statement} + languages: [lua] + destination: {type: implicit} + usePrePhraseSnapshot: false +spokenFormError: list insertion snippet +initialState: + documentContents: "" + selections: + - anchor: {line: 0, character: 0} + active: {line: 0, character: 0} + marks: {} +finalState: + documentContents: |- + if () { + + } + selections: + - anchor: {line: 0, character: 4} + active: {line: 0, character: 4} + thatMark: + - type: UntypedTarget + contentRange: + start: {line: 0, character: 0} + end: {line: 2, character: 1} + isReversed: false + hasExplicitRange: true diff --git a/packages/common/src/types/command/ActionDescriptor.ts b/packages/common/src/types/command/ActionDescriptor.ts index 549e7ea341..868aa7a4db 100644 --- a/packages/common/src/types/command/ActionDescriptor.ts +++ b/packages/common/src/types/command/ActionDescriptor.ts @@ -159,8 +159,9 @@ export interface CustomInsertSnippetArg { substitutions?: Record; } -interface ListInsertSnippetArg { +export interface ListInsertSnippetArg { type: "list"; + fallbackLanguage?: string; substitutions?: Record; snippets: CustomInsertSnippetArg[]; } @@ -190,8 +191,9 @@ export interface CustomWrapWithSnippetArg { languages?: string[]; } -interface ListWrapWithSnippetArg { +export interface ListWrapWithSnippetArg { type: "list"; + fallbackLanguage?: string; snippets: CustomWrapWithSnippetArg[]; } diff --git a/packages/common/src/types/snippet.types.ts b/packages/common/src/types/snippet.types.ts index 88777ae9de..a383920770 100644 --- a/packages/common/src/types/snippet.types.ts +++ b/packages/common/src/types/snippet.types.ts @@ -1,5 +1,7 @@ import type { SimpleScopeTypeType } from "./command/PartialTargetDescriptor.types"; +// FIXME: Legacy snippets + export interface SnippetScope { /** * VSCode language ids where this snippet definition should be active diff --git a/packages/cursorless-engine/src/core/compareSnippetDefinitions.ts b/packages/cursorless-engine/src/core/compareSnippetDefinitions.ts deleted file mode 100644 index 1b167f0ae7..0000000000 --- a/packages/cursorless-engine/src/core/compareSnippetDefinitions.ts +++ /dev/null @@ -1,93 +0,0 @@ -import type { - CustomInsertSnippetArg, - CustomWrapWithSnippetArg, - SimpleScopeTypeType, - SnippetScope, -} from "@cursorless/common"; - -/** - * Compares two snippet definitions by how specific their scope, breaking - * ties by origin. - * @param a One of the snippet definitions to compare - * @param b The other snippet definition to compare - * @returns A negative number if a should come before b, a positive number if b - */ -export function compareSnippetDefinitions< - T extends CustomInsertSnippetArg | CustomWrapWithSnippetArg, ->(a: T, b: T): number { - return compareSnippetScopes( - getScopeFromSnippetDescription(a), - getScopeFromSnippetDescription(b), - ); -} - -function getScopeFromSnippetDescription( - snippet: CustomInsertSnippetArg | CustomWrapWithSnippetArg, -): SnippetScope | undefined { - if (snippet.languages != null) { - return { - langIds: snippet.languages, - // Note what is called scopeTypes in the snippet arg is the - // insertion scope. Not scope to match against like with the - // function/method snippet example. - }; - } -} - -function compareSnippetScopes( - a: SnippetScope | undefined, - b: SnippetScope | undefined, -): number { - if (a == null && b == null) { - return 0; - } - - // Prefer the snippet that has a scope at all - if (a == null) { - return -1; - } - - if (b == null) { - return 1; - } - - // Prefer the snippet that is language-specific, regardless of scope type - if (a.langIds == null && b.langIds != null) { - return -1; - } - - if (b.langIds == null && a.langIds != null) { - return 1; - } - - // If both snippets are language-specific, prefer the snippet that specifies - // scope types. Note that this holds even if one snippet specifies more - // languages than the other. The motivating use case is if you have a snippet - // for functions in js and ts, and a snippet for methods in js and ts. If you - // override the function snippet for ts, you still want the method snippet to - // be used for ts methods. - const scopeTypesComparision = compareScopeTypes(a.scopeTypes, b.scopeTypes); - - if (scopeTypesComparision !== 0) { - return scopeTypesComparision; - } - - // If snippets both have scope types or both don't have scope types, prefer - // the snippet that specifies fewer languages - return a.langIds == null ? 0 : b.langIds!.length - a.langIds.length; -} - -function compareScopeTypes( - a: SimpleScopeTypeType[] | undefined, - b: SimpleScopeTypeType[] | undefined, -): number { - if (a == null && b != null) { - return -1; - } - - if (b == null && a != null) { - return 1; - } - - return 0; -} diff --git a/packages/cursorless-engine/src/core/getPreferredSnippet.test.ts b/packages/cursorless-engine/src/core/getPreferredSnippet.test.ts new file mode 100644 index 0000000000..ccd6d83684 --- /dev/null +++ b/packages/cursorless-engine/src/core/getPreferredSnippet.test.ts @@ -0,0 +1,176 @@ +import type { + CustomInsertSnippetArg, + CustomWrapWithSnippetArg, + ListInsertSnippetArg, + ListWrapWithSnippetArg, + NamedInsertSnippetArg, + NamedWrapWithSnippetArg, +} from "@cursorless/common"; +import assert from "node:assert"; +import { getPreferredSnippet } from "./getPreferredSnippet"; + +const insertNamed: NamedInsertSnippetArg = { + type: "named", + name: "named snippet", +}; + +const insertSingleLanguage: CustomInsertSnippetArg = { + type: "custom", + body: "custom body", + languages: ["a"], +}; + +const insertMultiLanguage: CustomInsertSnippetArg = { + type: "custom", + body: "custom body", + languages: ["a", "b"], +}; + +const insertGlobal: CustomInsertSnippetArg = { + type: "custom", + body: "custom body", +}; + +const insertionSnippets = [ + insertGlobal, + insertMultiLanguage, + insertSingleLanguage, +]; + +const wrapNamed: NamedWrapWithSnippetArg = { + type: "named", + name: "named wrap snippet", + variableName: "var", +}; + +const wrapSingleLanguage: CustomWrapWithSnippetArg = { + type: "custom", + body: "custom body", + variableName: "var", + languages: ["a"], +}; + +const wrapMultiLanguage: CustomWrapWithSnippetArg = { + type: "custom", + body: "custom body", + variableName: "var", + languages: ["a", "b"], +}; + +const wrapGlobal: CustomWrapWithSnippetArg = { + type: "custom", + body: "custom body", + variableName: "var", +}; + +const wrapSnippets = [wrapGlobal, wrapMultiLanguage, wrapSingleLanguage]; + +suite("getPreferredSnippet", () => { + test("Insert named", () => { + assert.throws(() => { + getPreferredSnippet(insertNamed, "a"); + }); + }); + + test("Insert single language", () => { + const snippet: ListInsertSnippetArg = { + type: "list", + fallbackLanguage: "b", + snippets: insertionSnippets, + }; + const preferred = getPreferredSnippet(snippet, "a"); + assert.equal(preferred, insertSingleLanguage); + }); + + test("Insert multi language", () => { + const snippet: ListInsertSnippetArg = { + type: "list", + snippets: insertionSnippets, + }; + const preferred = getPreferredSnippet(snippet, "b"); + assert.equal(preferred, insertMultiLanguage); + }); + + test("Insert fallback single language", () => { + const snippet: ListInsertSnippetArg = { + type: "list", + fallbackLanguage: "a", + snippets: insertionSnippets, + }; + const preferred = getPreferredSnippet(snippet, "c"); + assert.equal(preferred, insertSingleLanguage); + }); + + test("Insert fallback multi language", () => { + const snippet: ListInsertSnippetArg = { + type: "list", + fallbackLanguage: "b", + snippets: insertionSnippets, + }; + const preferred = getPreferredSnippet(snippet, "c"); + assert.equal(preferred, insertMultiLanguage); + }); + + test("Insert global", () => { + const snippet: ListInsertSnippetArg = { + type: "list", + snippets: insertionSnippets, + }; + const preferred = getPreferredSnippet(snippet, "c"); + assert.equal(preferred, insertGlobal); + }); + + test("Wrap named", () => { + assert.throws(() => { + getPreferredSnippet(wrapNamed, "a"); + }); + }); + + test("Wrap single language", () => { + const snippet: ListWrapWithSnippetArg = { + type: "list", + fallbackLanguage: "b", + snippets: wrapSnippets, + }; + const preferred = getPreferredSnippet(snippet, "a"); + assert.equal(preferred, wrapSingleLanguage); + }); + + test("Wrap multi language", () => { + const snippet: ListWrapWithSnippetArg = { + type: "list", + snippets: wrapSnippets, + }; + const preferred = getPreferredSnippet(snippet, "b"); + assert.equal(preferred, wrapMultiLanguage); + }); + + test("Wrap fallback single language", () => { + const snippet: ListWrapWithSnippetArg = { + type: "list", + fallbackLanguage: "a", + snippets: wrapSnippets, + }; + const preferred = getPreferredSnippet(snippet, "c"); + assert.equal(preferred, wrapSingleLanguage); + }); + + test("Wrap fallback multi language", () => { + const snippet: ListWrapWithSnippetArg = { + type: "list", + fallbackLanguage: "b", + snippets: wrapSnippets, + }; + const preferred = getPreferredSnippet(snippet, "c"); + assert.equal(preferred, wrapMultiLanguage); + }); + + test("Wrap global", () => { + const snippet: ListWrapWithSnippetArg = { + type: "list", + snippets: wrapSnippets, + }; + const preferred = getPreferredSnippet(snippet, "c"); + assert.equal(preferred, wrapGlobal); + }); +}); diff --git a/packages/cursorless-engine/src/core/getPreferredSnippet.ts b/packages/cursorless-engine/src/core/getPreferredSnippet.ts index 3c84bfbb50..5dbc092de4 100644 --- a/packages/cursorless-engine/src/core/getPreferredSnippet.ts +++ b/packages/cursorless-engine/src/core/getPreferredSnippet.ts @@ -4,7 +4,6 @@ import type { InsertSnippetArg, WrapWithSnippetArg, } from "@cursorless/common"; -import { compareSnippetDefinitions } from "./compareSnippetDefinitions"; export function getPreferredSnippet( snippetDescription: InsertSnippetArg, @@ -29,12 +28,11 @@ export function getPreferredSnippet( return snippetDescription; } - const filteredSnippets = filterSnippetDefinitions( + const preferredSnippet = tryToFindPreferredSnippet( snippetDescription.snippets, languageId, + snippetDescription.fallbackLanguage, ); - filteredSnippets.sort(compareSnippetDefinitions); - const preferredSnippet = filteredSnippets[0]; if (preferredSnippet == null) { const languages = getUniqueLanguagesString(snippetDescription.snippets); @@ -53,18 +51,53 @@ function getUniqueLanguagesString(snippets: CustomInsertSnippetArg[]): string { return Array.from(languages).sort().join(", "); } -/** - * Filter snippet definitions by language. - * @param snippetDescriptions The snippets to filter - * @returns The snippets that are relevant to the current language - */ -function filterSnippetDefinitions< +function tryToFindPreferredSnippet< T extends CustomInsertSnippetArg | CustomWrapWithSnippetArg, ->(snippetDescriptions: T[], languageId: string): T[] { - return snippetDescriptions.filter((snippetDescription) => { - if (snippetDescription.languages == null) { - return true; +>( + snippetDescriptions: T[], + languageId: string, + fallbackLanguage: string | undefined, +): T | undefined { + // First try to find snippet matching language id + let snippet = findSnippetWithFewestLanguages( + snippetDescriptions.filter((snippetDescription) => { + return snippetDescription.languages?.includes(languageId); + }), + ); + + // Secondly try to find snippet matching fallback language + if (snippet == null && fallbackLanguage != null) { + snippet = findSnippetWithFewestLanguages( + snippetDescriptions.filter((snippetDescription) => { + return snippetDescription.languages?.includes(fallbackLanguage); + }), + ); + } + + // Finally try to find global snippet + if (snippet == null) { + snippet = snippetDescriptions.find((snippetDescription) => { + return snippetDescription.languages == null; + }); + } + + return snippet; +} + +function findSnippetWithFewestLanguages< + T extends CustomInsertSnippetArg | CustomWrapWithSnippetArg, +>(snippets: T[]): T | undefined { + if (snippets.length === 0) { + return undefined; + } + + // Find the snippet with the fewest languages + return snippets.reduce((prev, curr) => { + if (prev.languages == null || curr.languages == null) { + throw Error( + "Snippet must have languages defined to find the one with the fewest languages", + ); } - return snippetDescription.languages.includes(languageId); + return curr.languages.length < prev.languages.length ? curr : prev; }); }