-
Notifications
You must be signed in to change notification settings - Fork 54
Fix action.yml snippets to replace typed text instead of inserting #307
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
| // Get action scaffolding snippets if applicable | ||
| let actionSnippets: CompletionItem[] = []; | ||
| if (isAction && config?.featureFlags?.isEnabled("actionScaffoldingSnippets")) { | ||
| actionSnippets = getActionScaffoldingSnippets(parsedTemplate.value, path, position, replaceRange); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moved down because it needs replaceRange
b6b084c to
af7f08e
Compare
| path: TemplateToken[], | ||
| position: Position | ||
| position: Position, | ||
| replaceRange?: Range |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
plumbing replaceRange. Refer languageservice/src/complete.ts
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is consistent with now non-snippet completion works
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Fixes action scaffolding snippet completions so selecting a snippet replaces the user’s already-typed text (prefix) instead of inserting the snippet after it.
Changes:
- Compute and pass a
replaceRangeinto action scaffolding snippet generation. - Update scaffolding snippet
textEditto useTextEdit.replace(...)when a replace range is available. - Add tests asserting scaffolding snippets replace typed prefixes.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| languageservice/src/complete.ts | Moves snippet generation to after replaceRange calculation and passes replaceRange into scaffolding snippet creation. |
| languageservice/src/complete-action.ts | Extends scaffolding snippet helpers to accept an optional replaceRange and uses it to create replace-vs-insert textEdits. |
| languageservice/src/complete-action.test.ts | Adds test coverage ensuring scaffolding snippets replace typed prefixes and handle empty-file scenarios. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const [doc, position] = createActionDocument(`|`); | ||
| const completions = await complete(doc, position, scaffoldingConfig); | ||
|
|
||
| const compositeSnippet = completions.find(c => c.label === "Composite Action"); |
Copilot
AI
Jan 21, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In this test, compositeSnippet isn't asserted to exist before dereferencing textEdit.range, so a missing snippet will fail with a TypeError rather than a clear expectation failure. Add an explicit expect(compositeSnippet).toBeDefined() (or assert on compositeSnippet?.textEdit) before reading textEdit.range.* to improve debuggability.
| const compositeSnippet = completions.find(c => c.label === "Composite Action"); | |
| const compositeSnippet = completions.find(c => c.label === "Composite Action"); | |
| expect(compositeSnippet).toBeDefined(); |
No description provided.