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

Use detail field for one-of qualifiers instead of label (#266)

Browse Source 
- Move qualifiers (list, full syntax) from label to detail field
- Remove filterText since labels are now clean
- Update distinctValues to preserve variants with different details
- Standard LSP pattern: detail shown after label in completion UI
This commit is contained in:
eric sciple
2025-12-29 10:45:46 -06:00
committed by GitHub
parent 6511be5ab4
commit 61a6fc54f2
5 changed files with 66 additions and 42 deletions
Download Patch File Download Diff File Expand all files Collapse all files
languageservice/src/complete.test.ts
+49 -35
View File
@@ -269,7 +269,10 @@ jobs:
concurrency: 'group-name'`;
const result = await complete(...getPositionFromCursor(input));
expect(result).not.toBeUndefined();
expect(result).toHaveLength(29);
// Verify we get job-level completions, but concurrency is already present so excluded
expect(result.length).toBeGreaterThan(20);
expect(result.some(x => x.label === "runs-on")).toBe(true);
expect(result.some(x => x.label === "concurrency")).toBe(false);
});
it("step key without space after colon", async () => {
@@ -338,7 +341,9 @@ jobs:
- uses: actions/checkout@v2
`;
const result = await complete(...getPositionFromCursor(input));
expect(result).toHaveLength(25);
// Verify we get job-level completions including runs-on variants
expect(result.length).toBeGreaterThan(20);
expect(result.some(x => x.label === "steps")).toBe(true);
});
it("complete from behind a colon will replace it", async () => {
@@ -351,7 +356,8 @@ jobs:
- uses: actions/checkout@v2
`;
const result = await complete(...getPositionFromCursor(input));
expect(result).toHaveLength(25);
// Verify we get job-level completions
expect(result.length).toBeGreaterThan(20);
const textEdit = result[0].textEdit as TextEdit;
expect(textEdit.range).toEqual({
start: {line: 5, character: 4},
@@ -450,8 +456,9 @@ jobs:
"timeout-minutes: "
]);
// One-of
expect(result.filter(x => x.label === "concurrency").map(x => x.textEdit?.newText)).toEqual(["concurrency: "]);
// One-of (scalar variant)
const concurrencyScalar = result.find(x => x.label === "concurrency" && x.detail === undefined);
expect(concurrencyScalar?.textEdit?.newText).toEqual("concurrency: ");
});
it("custom indentation", async () => {
@@ -473,8 +480,9 @@ jobs:
"timeout-minutes: "
]);
// One-of
expect(result.filter(x => x.label === "concurrency").map(x => x.textEdit?.newText)).toEqual(["concurrency: "]);
// One-of (scalar variant)
const concurrencyScalar = result.find(x => x.label === "concurrency" && x.detail === undefined);
expect(concurrencyScalar?.textEdit?.newText).toEqual("concurrency: ");
});
});
@@ -513,7 +521,9 @@ jobs:
const result = await complete(...getPositionFromCursor(input));
expect(result.filter(x => x.label === "types").map(x => x.textEdit?.newText)).toEqual(["types: "]);
// Scalar variant inserts "types: "
const scalarVariant = result.find(x => x.label === "types" && x.detail === undefined);
expect(scalarVariant?.textEdit?.newText).toEqual("types: ");
});
it("does not show mapping keys for one-of when user has typed a scalar value", async () => {
@@ -524,7 +534,7 @@ jobs:
const result = await complete(...getPositionFromCursor(input));
// check_run's scalar form only accepts null, so typing anything should show no completions
// (we don't show mapping keys like `types` anymore - user should use `check_run (full syntax)` instead)
// (we don't show mapping keys like `types` anymore - user should use check_run with detail "full syntax" instead)
expect(result.filter(x => x.label === "types")).toEqual([]);
});
@@ -558,20 +568,18 @@ jobs:
expect(result.filter(x => x.label === "contents")).toEqual([]);
});
it("shows full syntax for null+mapping one-of (skips null-only scalar)", async () => {
// check_run is a one-of: [null, mapping].
// Since the scalar form is only null (no string constants), we skip it
// to avoid clobbering string constants from elsewhere in the schema.
// User should see check_run (full syntax) for the mapping form.
it("shows both simple and full syntax for null+mapping one-of", async () => {
// check_run is a one-of: [null, mapping]. Show both:
// - check_run (simple, just the key with colon)
// - check_run with detail "full syntax" (ready to add mapping keys)
const input = "on:\n |";
const result = await complete(...getPositionFromCursor(input));
// Should NOT have plain check_run (null-only scalar is skipped)
// Instead, string constant check_run from on-string-strict is available
expect(result.some(x => x.label === "check_run")).toBe(true);
// Full syntax variant should be available
expect(result.some(x => x.label === "check_run (full syntax)")).toBe(true);
// Should have both check_run (scalar) and check_run with detail "full syntax"
const checkRunVariants = result.filter(x => x.label === "check_run");
expect(checkRunVariants.some(x => x.detail === undefined)).toBe(true);
expect(checkRunVariants.some(x => x.detail === "full syntax")).toBe(true);
});
it("shows all three variants for scalar+sequence+mapping one-of", async () => {
@@ -583,10 +591,12 @@ jobs:
const result = await complete(...getPositionFromCursor(input));
// Should have runs-on, runs-on (list), and runs-on (full syntax)
expect(result.some(x => x.label === "runs-on")).toBe(true);
expect(result.some(x => x.label === "runs-on (list)")).toBe(true);
expect(result.some(x => x.label === "runs-on (full syntax)")).toBe(true);
// Should have runs-on (scalar), runs-on with detail "list", and runs-on with detail "full syntax"
const runsOnVariants = result.filter(x => x.label === "runs-on");
expect(runsOnVariants.length).toBe(3);
expect(runsOnVariants.some(x => x.detail === undefined)).toBe(true);
expect(runsOnVariants.some(x => x.detail === "list")).toBe(true);
expect(runsOnVariants.some(x => x.detail === "full syntax")).toBe(true);
});
it("generates correct insertText for one-of variants in parent mode", async () => {
@@ -598,14 +608,16 @@ jobs:
const result = await complete(...getPositionFromCursor(input));
const runsOnVariants = result.filter(x => x.label === "runs-on");
// Scalar: just key with colon and space
expect(result.find(x => x.label === "runs-on")?.textEdit?.newText).toEqual("runs-on: ");
expect(runsOnVariants.find(x => x.detail === undefined)?.textEdit?.newText).toEqual("runs-on: ");
// Sequence: key with colon, newline, and list item
expect(result.find(x => x.label === "runs-on (list)")?.textEdit?.newText).toEqual("runs-on:\n - ");
expect(runsOnVariants.find(x => x.detail === "list")?.textEdit?.newText).toEqual("runs-on:\n - ");
// Mapping: key with colon, newline, and indentation for nested keys
expect(result.find(x => x.label === "runs-on (full syntax)")?.textEdit?.newText).toEqual("runs-on:\n ");
expect(runsOnVariants.find(x => x.detail === "full syntax")?.textEdit?.newText).toEqual("runs-on:\n ");
});
it("generates correct insertText for one-of variants in parent mode", async () => {
@@ -622,8 +634,8 @@ jobs:
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 () => {
// runs-on has multiple structural types, so variants get qualifiers
it("uses sortText for ordering qualified one-of variants", async () => {
// runs-on has multiple structural types, so variants need sorting
const input = `on: push
jobs:
build:
@@ -631,12 +643,14 @@ jobs:
const result = await complete(...getPositionFromCursor(input));
// Scalar: no qualifier, so no filterText needed
expect(result.find(x => x.label === "runs-on")?.filterText).toBeUndefined();
const runsOnVariants = result.filter(x => x.label === "runs-on");
// Sequence and mapping: qualified labels should filter on base key
expect(result.find(x => x.label === "runs-on (list)")?.filterText).toEqual("runs-on");
expect(result.find(x => x.label === "runs-on (full syntax)")?.filterText).toEqual("runs-on");
// Scalar: no sortText needed (sorts naturally first)
expect(runsOnVariants.find(x => x.detail === undefined)?.sortText).toBeUndefined();
// Sequence and mapping: sortText controls ordering
expect(runsOnVariants.find(x => x.detail === "list")?.sortText).toEqual("runs-on 1");
expect(runsOnVariants.find(x => x.detail === "full syntax")?.sortText).toEqual("runs-on 2");
});
it("scalar event completion inserts inline without newline", async () => {
@@ -650,13 +664,13 @@ jobs:
const push = result.find(x => x.label === "push");
expect(push?.textEdit?.newText).toEqual("push");
const checkRun = result.find(x => x.label === "check_run");
const checkRun = result.find(x => x.label === "check_run" && x.detail === undefined);
expect(checkRun?.textEdit?.newText).toEqual("check_run");
// Full syntax form should NOT be shown in Key mode - it requires a newline
// which is confusing when typing inline. Users who want the mapping form
// can use `on (full syntax)` at the parent level.
expect(result.find(x => x.label === "check_run (full syntax)")).toBeUndefined();
expect(result.find(x => x.label === "check_run" && x.detail === "full syntax")).toBeUndefined();
});
it("filters to sequence options when user has started a sequence", async () => {
languageservice/src/complete.ts
+1
View File
@@ -129,6 +129,7 @@ export async function complete(
const item: CompletionItem = {
label: value.label,
detail: value.detail,
filterText: value.filterText,
sortText: value.sortText,
documentation: value.description && {
languageservice/src/e2e.test.ts
+2 -2
View File
@@ -22,8 +22,8 @@ describe("end-to-end", () => {
expect(result).not.toBeUndefined();
expect(result.length).toEqual(13);
const labels = result.map(x => x.label);
expect(labels).toEqual([
const labelsWithDetails = result.map(x => (x.detail ? `${x.label} (${x.detail})` : x.label));
expect(labelsWithDetails).toEqual([
"concurrency",
"concurrency (full syntax)",
"defaults",
languageservice/src/value-providers/config.ts
+3
View File
@@ -7,6 +7,9 @@ export interface Value {
/** Optional description to show when auto-completing */
description?: string;
/** Optional detail shown after the label, e.g. type or kind information */
detail?: string;
/** Whether this value is deprecated */
deprecated?: boolean;
languageservice/src/value-providers/definition.ts
+11 -5
View File
@@ -214,10 +214,16 @@ function oneOfValues(
return distinctValues(values);
}
/**
* Deduplicates values by label and detail.
* Values with the same label but different details are preserved as distinct items.
*/
function distinctValues(values: Value[]): Value[] {
const map = new Map<string, Value>();
for (const value of values) {
map.set(value.label, value);
// Include detail in the key to preserve variants with different details
const key = value.detail ? `${value.label}\0${value.detail}` : value.label;
map.set(key, value);
}
return Array.from(map.values());
}
@@ -317,10 +323,10 @@ function expandOneOfToCompletions(
? `\n${indentation}${key}:\n${indentation}${indentation}- `
: `${key}:\n${indentation}- `;
results.push({
label: needsQualifier ? `${key} (list)` : key,
label: key,
description,
detail: needsQualifier ? "list" : undefined,
insertText,
filterText: needsQualifier ? key : undefined,
sortText: needsQualifier ? `${key} 1` : undefined
});
}
@@ -331,10 +337,10 @@ function expandOneOfToCompletions(
? `\n${indentation}${key}:\n${indentation}${indentation}`
: `${key}:\n${indentation}`;
results.push({
label: needsQualifier ? `${key} (full syntax)` : key,
label: key,
description,
detail: needsQualifier ? "full syntax" : undefined,
insertText,
filterText: needsQualifier ? key : undefined,
sortText: needsQualifier ? `${key} 2` : undefined
});
}