actions/languageservices
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Fix autocomplete showing mapping keys for empty values (#268)

Browse Source 
Follow-up to #265

When completing an empty value (e.g., `permissions: |`), mapping keys were
incorrectly shown alongside scalar values. This made completions confusing.

Before:
- `permissions: |` showed read-all, write-all, AND actions, contents, etc.
- `on: |` showed check_run AND check_run (full syntax), etc.

After:
- `permissions: |` shows only read-all and write-all
- `on: |` shows only event names like push, check_run
- `concurrency: |` shows no completions (user types their own group name)

Users who want the mapping form choose (full syntax) completions at the
parent level.
This commit is contained in:
eric sciple
2025-12-29 09:50:59 -06:00
committed by GitHub
parent a06ceee92b
commit 6511be5ab4
2 changed files with 45 additions and 29 deletions
Download Patch File Download Diff File Expand all files Collapse all files
languageservice/src/complete.test.ts
+34 -29
View File
@@ -212,7 +212,9 @@ jobs:
expect(result[0].label).toEqual("my-custom-label"); expect(result[0].label).toEqual("my-custom-label");
}); });
it("does not show parent mapping sibling keys", async () => { it("does not show mapping keys or parent sibling keys in Key mode", async () => {
// At `container: |`, the scalar form is a string with no constants.
// Mapping keys should NOT be shown - users should use `container (full syntax)`.
const input = `on: push const input = `on: push
jobs: jobs:
build: build:
@@ -220,20 +222,21 @@ jobs:
runs-on: ubuntu-latest`; runs-on: ubuntu-latest`;
const result = await complete(...getPositionFromCursor(input)); const result = await complete(...getPositionFromCursor(input));
expect(result).not.toBeUndefined(); expect(result).not.toBeUndefined();
expect(result.length).toEqual(6); // No completions because: scalar has no constants, mapping variant skipped in Key mode
// Should not contain other top-level job keys like `if` and `runs-on` expect(result.length).toEqual(0);
expect(result.map(x => x.label)).not.toContain("if");
expect(result.map(x => x.label)).not.toContain("runs-on");
}); });
it("shows mapping keys within a new map ", async () => { it("does not show mapping keys in Key mode when structure is uncommitted", async () => {
// At `concurrency: |`, user is in Key mode but hasn't committed to a structure.
// The scalar form is a string with no constants, so no completions.
// Mapping keys are NOT shown - users should use `concurrency (full syntax)` at parent level.
const input = `on: push const input = `on: push
jobs: jobs:
build: build:
concurrency: |`; concurrency: |`;
const result = await complete(...getPositionFromCursor(input)); const result = await complete(...getPositionFromCursor(input));
expect(result).not.toBeUndefined(); expect(result).not.toBeUndefined();
expect(result.map(x => x.label).sort()).toEqual(["cancel-in-progress", "group"]); expect(result.map(x => x.label)).toEqual([]);
}); });
it("job key", async () => { it("job key", async () => {
@@ -475,15 +478,15 @@ jobs:
}); });
}); });
it("adds a new line and indentation for mapping keys when the key is given", async () => { it("does not show mapping keys in Key mode for one-of with mapping variant", async () => {
// At `concurrency: |`, mapping keys should NOT be shown.
// Users who want the mapping form should use `concurrency (full syntax)` at parent level.
const input = "concurrency: |"; const input = "concurrency: |";
const result = await complete(...getPositionFromCursor(input)); const result = await complete(...getPositionFromCursor(input));
expect(result.filter(x => x.label === "cancel-in-progress").map(x => x.textEdit?.newText)).toEqual([ expect(result.filter(x => x.label === "cancel-in-progress")).toEqual([]);
"\n cancel-in-progress: " expect(result.filter(x => x.label === "group")).toEqual([]);
]);
expect(result.filter(x => x.label === "group").map(x => x.textEdit?.newText)).toEqual(["\n group: "]);
}); });
it("does not add new line if no key in line", async () => { it("does not add new line if no key in line", async () => {
@@ -525,8 +528,10 @@ jobs:
expect(result.filter(x => x.label === "types")).toEqual([]); expect(result.filter(x => x.label === "types")).toEqual([]);
}); });
it("shows all options for one-of when user hasn't committed to a type yet", async () => { it("shows only scalar options for one-of in Key mode when user hasn't committed to a type", async () => {
// At `permissions: |` user hasn't typed anything yet - show all options // At `permissions: |` user hasn't typed anything yet - show only scalar options
// Mapping keys are NOT shown because they would require a newline
// Users who want the mapping form can use `permissions (full syntax)` at the parent level
const input = "on: push\npermissions: |"; const input = "on: push\npermissions: |";
const result = await complete(...getPositionFromCursor(input)); const result = await complete(...getPositionFromCursor(input));
@@ -535,9 +540,9 @@ jobs:
expect(result.filter(x => x.label === "read-all").map(x => x.textEdit?.newText)).toEqual(["read-all"]); expect(result.filter(x => x.label === "read-all").map(x => x.textEdit?.newText)).toEqual(["read-all"]);
expect(result.filter(x => x.label === "write-all").map(x => x.textEdit?.newText)).toEqual(["write-all"]); expect(result.filter(x => x.label === "write-all").map(x => x.textEdit?.newText)).toEqual(["write-all"]);
// Mapping keys should also be available (user hasn't committed yet) // Mapping keys should NOT be shown - they require a newline which is confusing inline
expect(result.filter(x => x.label === "actions").map(x => x.textEdit?.newText)).toEqual(["\n actions: "]); expect(result.filter(x => x.label === "actions")).toEqual([]);
expect(result.filter(x => x.label === "contents").map(x => x.textEdit?.newText)).toEqual(["\n contents: "]); expect(result.filter(x => x.label === "contents")).toEqual([]);
}); });
it("filters to scalar options when user has started typing a scalar", async () => { it("filters to scalar options when user has started typing a scalar", async () => {
@@ -603,17 +608,18 @@ jobs:
expect(result.find(x => x.label === "runs-on (full syntax)")?.textEdit?.newText).toEqual("runs-on:\n "); expect(result.find(x => x.label === "runs-on (full syntax)")?.textEdit?.newText).toEqual("runs-on:\n ");
}); });
it("generates correct insertText for one-of variants in key mode", async () => { it("generates correct insertText for one-of variants in parent mode", async () => {
// concurrency is a one-of: [string, mapping] - testing key mode (after colon on same line) // concurrency is a one-of: [string, mapping] - testing parent mode (inside mapping)
const input = "concurrency: |"; // At `concurrency:\n |`, user HAS committed to mapping structure, so mapping keys are shown
const input = "concurrency:\n |";
const result = await complete(...getPositionFromCursor(input)); const result = await complete(...getPositionFromCursor(input));
// Scalar in key mode: newline + indented key + colon + space // In parent mode: just key + colon + space (no leading newline)
expect(result.find(x => x.label === "group")?.textEdit?.newText).toEqual("\n group: "); expect(result.find(x => x.label === "group")?.textEdit?.newText).toEqual("group: ");
// Boolean in key mode (cancel-in-progress): newline + indented key + colon + space // Boolean in parent mode (cancel-in-progress): key + colon + space
expect(result.find(x => x.label === "cancel-in-progress")?.textEdit?.newText).toEqual("\n cancel-in-progress: "); expect(result.find(x => x.label === "cancel-in-progress")?.textEdit?.newText).toEqual("cancel-in-progress: ");
}); });
it("uses base key as filterText for qualified one-of variants", async () => { it("uses base key as filterText for qualified one-of variants", async () => {
@@ -647,11 +653,10 @@ jobs:
const checkRun = result.find(x => x.label === "check_run"); const checkRun = result.find(x => x.label === "check_run");
expect(checkRun?.textEdit?.newText).toEqual("check_run"); expect(checkRun?.textEdit?.newText).toEqual("check_run");
// Full syntax form inserts as a mapping key (with newline in Key mode) // Full syntax form should NOT be shown in Key mode - it requires a newline
// This is expected behavior - it starts the mapping form // which is confusing when typing inline. Users who want the mapping form
const checkRunFull = result.find(x => x.label === "check_run (full syntax)"); // can use `on (full syntax)` at the parent level.
// In Key mode: \n + indent + key + : + \n + indent + indent (for nested content) expect(result.find(x => x.label === "check_run (full syntax)")).toBeUndefined();
expect(checkRunFull?.textEdit?.newText).toEqual("\n check_run:\n ");
}); });
it("filters to sequence options when user has started a sequence", async () => { it("filters to sequence options when user has started a sequence", async () => {
languageservice/src/value-providers/definition.ts
+11
View File
@@ -198,6 +198,17 @@ function oneOfValues(
} }
} }
// In Key mode (after colon, e.g., `on: |`), only include scalar variants when
// completing an empty value. Mapping/sequence forms require newlines which is
// confusing when typing inline. Users who want those forms can use completions
// like `(full syntax)` or `(list)` at the parent level.
if (!tokenStructure && mode === DefinitionValueMode.Key) {
const variantBucket = getStructuralBucket(variantDef.definitionType);
if (variantBucket !== "scalar") {
continue;
}
}
values.push(...definitionValues(variantDef, indentation, mode, tokenStructure)); values.push(...definitionValues(variantDef, indentation, mode, tokenStructure));
} }
return distinctValues(values); return distinctValues(values);