From 79145efdbc0e38b01ac4a58c5e47e7fd496ecdbf Mon Sep 17 00:00:00 2001 From: William Killerud Date: Sun, 21 Jan 2024 11:38:49 +0100 Subject: [PATCH 1/5] test: add tests for built-ins insertText These suggestions have the same quirk as other namespaces, where we end up with an extra dot in Vuelike contexts. --- e2e/src/suite/completion/completion.test.ts | 34 ++++++++++++++++++--- e2e/src/suite/completion/helper.ts | 20 ++++++------ fixtures/e2e/completion/AppButton.astro | 6 ++++ fixtures/e2e/completion/AppButton.svelte | 5 +++ fixtures/e2e/completion/AppButton.vue | 5 +++ fixtures/e2e/completion/main.scss | 6 ++++ 6 files changed, 61 insertions(+), 15 deletions(-) diff --git a/e2e/src/suite/completion/completion.test.ts b/e2e/src/suite/completion/completion.test.ts index 2e6135f1..96433e89 100644 --- a/e2e/src/suite/completion/completion.test.ts +++ b/e2e/src/suite/completion/completion.test.ts @@ -194,16 +194,16 @@ describe("SCSS Completion Test", function () { // The insertText must be without one to avoid $$color. However, filterText // still need the $ sign for the suggestion to match. let expectedCompletions = [ - { label: "$color", insertText: '"color"', filterText: undefined }, - { label: "$fonts", insertText: '"fonts"', filterText: undefined }, + { label: "$color", insertText: '"color"' }, + { label: "$fonts", insertText: '"fonts"' }, ]; await testCompletion(vueDocUri, position(16, 11), expectedCompletions); await testCompletion(astroDocUri, position(11, 11), expectedCompletions); expectedCompletions = [ - { label: "$color", insertText: '"$color"', filterText: undefined }, - { label: "$fonts", insertText: '"$fonts"', filterText: undefined }, + { label: "$color", insertText: '"$color"' }, + { label: "$fonts", insertText: '"$fonts"' }, ]; await testCompletion(svelteDocUri, position(8, 11), expectedCompletions); @@ -243,6 +243,32 @@ describe("SCSS Completion Test", function () { await testCompletion(astroDocUri, position(31, 40), expectedCompletions); }); + it("Offers completions for Sass built-ins", async () => { + let expectedCompletions = [ + { + label: "floor", + insertText: '".floor(${1:number})"', + filterText: '"math.floor"', + }, + ]; + + await testCompletion(docUri, position(36, 19), expectedCompletions); + + // For Vue, Svelte and Astro, the existing . from the namespace is not replaced by VS Code, so omit them from insertText. + // However, we still need them both in the filter text. + expectedCompletions = [ + { + label: "floor", + insertText: '"floor(${1:number})"', + filterText: '"math.floor"', + }, + ]; + + await testCompletion(vueDocUri, position(42, 19), expectedCompletions); + await testCompletion(svelteDocUri, position(34, 19), expectedCompletions); + await testCompletion(astroDocUri, position(37, 19), expectedCompletions); + }); + it("Offers namespace completion inside string interpolation with preceeding non-space character", async () => { const expectedCompletions = [ { diff --git a/e2e/src/suite/completion/helper.ts b/e2e/src/suite/completion/helper.ts index afa783fe..e6534eb5 100644 --- a/e2e/src/suite/completion/helper.ts +++ b/e2e/src/suite/completion/helper.ts @@ -48,6 +48,14 @@ export async function testCompletion( } } else { const match = result.items.find((i) => { + if (Object.prototype.hasOwnProperty.call(ei, "filterText")) { + if (JSON.stringify(i.filterText) === ei.filterText) { + return true; + } else { + return false; + } + } + if (typeof i.label === "string") { return i.label === ei.label; } @@ -76,6 +84,7 @@ export async function testCompletion( if (ei.detail) { assert.strictEqual(match.detail, ei.detail); } + if (ei.insertText) { assert.ok( JSON.stringify(match.insertText).includes(ei.insertText), @@ -85,17 +94,6 @@ export async function testCompletion( ); } - // This may deliberatly be undefined, in which case the filter matches the label - if (Object.prototype.hasOwnProperty.call(ei, "filterText")) { - assert.strictEqual( - JSON.stringify(match.filterText), - ei.filterText, - `Expected filterText to match ${ - ei.filterText - }. Actual: ${JSON.stringify(match.filterText)}`, - ); - } - if (ei.documentation) { if (typeof match.documentation === "string") { assert.strictEqual(match.documentation, ei.documentation); diff --git a/fixtures/e2e/completion/AppButton.astro b/fixtures/e2e/completion/AppButton.astro index b6a29eb4..54403d45 100644 --- a/fixtures/e2e/completion/AppButton.astro +++ b/fixtures/e2e/completion/AppButton.astro @@ -30,4 +30,10 @@ $fonts: -apple-system; @include ns. --runtime-var: var(--other-var, #{ns.}) } + +@use "sass:math"; + +.foo { + padding: math.fl +} diff --git a/fixtures/e2e/completion/AppButton.svelte b/fixtures/e2e/completion/AppButton.svelte index f655b836..2f59a58e 100644 --- a/fixtures/e2e/completion/AppButton.svelte +++ b/fixtures/e2e/completion/AppButton.svelte @@ -28,4 +28,9 @@ $fonts: -apple-system; --runtime-var: var(--other-var, #{ns.}) } +@use "sass:math"; + +.foo { + padding: math.fl +} diff --git a/fixtures/e2e/completion/AppButton.vue b/fixtures/e2e/completion/AppButton.vue index ad910b98..d5b6c970 100644 --- a/fixtures/e2e/completion/AppButton.vue +++ b/fixtures/e2e/completion/AppButton.vue @@ -36,4 +36,9 @@ $fonts: -apple-system; --runtime-var: var(--other-var, #{ns.}) } +@use "sass:math"; + +.foo { + padding: math.fl +} diff --git a/fixtures/e2e/completion/main.scss b/fixtures/e2e/completion/main.scss index aa61c818..c28f4674 100644 --- a/fixtures/e2e/completion/main.scss +++ b/fixtures/e2e/completion/main.scss @@ -29,3 +29,9 @@ $fonts: -apple-system; @function _multiply($value) { @return $value * ns.; } + +@use "sass:math"; + +.foo { + padding: math.fl +} From 43a8d80b2c644c5bb6a6b92d64b2fa164dbc801c Mon Sep 17 00:00:00 2001 From: William Killerud Date: Sun, 21 Jan 2024 11:39:55 +0100 Subject: [PATCH 2/5] fix: suggest built-in function with signature also when there's text after the dot --- server/src/features/completion/completion.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/features/completion/completion.ts b/server/src/features/completion/completion.ts index 99010b7b..fd2069c0 100644 --- a/server/src/features/completion/completion.ts +++ b/server/src/features/completion/completion.ts @@ -215,7 +215,7 @@ function doBuiltInCompletion( return { label: name, filterText: `${context.namespace}.${name}`, - insertText: context.word.endsWith(".") + insertText: context.word.includes(".") ? `.${name}${signature ? `(${parameterSnippet})` : ""}` : name, insertTextFormat: parameterSnippet From fddd8f5350dbbcf5eaa2539998f3a0e0ac04edc7 Mon Sep 17 00:00:00 2001 From: William Killerud Date: Sun, 21 Jan 2024 11:51:49 +0100 Subject: [PATCH 3/5] fix: remove double dots for Sass built-ins in Svelte, Vue, Astro --- server/src/features/completion/completion.ts | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/server/src/features/completion/completion.ts b/server/src/features/completion/completion.ts index fd2069c0..648b2309 100644 --- a/server/src/features/completion/completion.ts +++ b/server/src/features/completion/completion.ts @@ -212,12 +212,23 @@ function doBuiltInCompletion( ): void { completions.items = Object.entries(module.exports).map( ([name, { description, signature, parameterSnippet, returns }]) => { + // Client needs the namespace as part of the text that is matched, + const filterText = `${context.namespace}.${name}`; + + // Inserted text needs to include the `.` which will otherwise + // be replaced (except when we're embedded in Vue, Svelte or Astro). + // Example result: .floor(${1:number}) + const isEmbedded = context.originalExtension !== "scss"; + const insertText = context.word.includes(".") + ? `${isEmbedded ? "" : "."}${name}${ + signature ? `(${parameterSnippet})` : "" + }` + : name; + return { label: name, - filterText: `${context.namespace}.${name}`, - insertText: context.word.includes(".") - ? `.${name}${signature ? `(${parameterSnippet})` : ""}` - : name, + filterText, + insertText, insertTextFormat: parameterSnippet ? InsertTextFormat.Snippet : InsertTextFormat.PlainText, From 7822e2460a7e9b8b74e28c93f50d9cec7a199c19 Mon Sep 17 00:00:00 2001 From: William Killerud Date: Sun, 21 Jan 2024 11:53:57 +0100 Subject: [PATCH 4/5] test: add completions test for Sass built-ins for web --- web/src/suite/completion.test.ts | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/web/src/suite/completion.test.ts b/web/src/suite/completion.test.ts index 39a7fedc..63d7f958 100644 --- a/web/src/suite/completion.test.ts +++ b/web/src/suite/completion.test.ts @@ -175,6 +175,18 @@ describe("Completions", () => { await testCompletion(docUri, position(25, 40), expectedCompletions); }); + it("for Sass built-ins", async () => { + const expectedCompletions = [ + { + label: "floor", + insertText: '".floor(${1:number})"', + filterText: '"math.floor"', + }, + ]; + + await testCompletion(docUri, position(36, 19), expectedCompletions); + }); + it("inside string interpolation with preceeding non-space character", async () => { const expectedCompletions = [ { From 649d140cd14193d415cd105b1ca82fb56bc16419 Mon Sep 17 00:00:00 2001 From: William Killerud Date: Sun, 21 Jan 2024 12:09:42 +0100 Subject: [PATCH 5/5] chore: run linter with --fix --- server/src/features/completion/completion.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/features/completion/completion.ts b/server/src/features/completion/completion.ts index 648b2309..cabcb42c 100644 --- a/server/src/features/completion/completion.ts +++ b/server/src/features/completion/completion.ts @@ -222,7 +222,7 @@ function doBuiltInCompletion( const insertText = context.word.includes(".") ? `${isEmbedded ? "" : "."}${name}${ signature ? `(${parameterSnippet})` : "" - }` + }` : name; return {