Skip to content

fix: honor completion snippet support capability - #1346

Open
Shubham-Padkonde wants to merge 2 commits into
redhat-developer:mainfrom
Shubham-Padkonde:fix/completion-snippet-capability
Open

Shubham-Padkonde wants to merge 2 commits into
redhat-developer:mainfrom
Shubham-Padkonde:fix/completion-snippet-capability

Conversation

@Shubham-Padkonde

@Shubham-Padkonde Shubham-Padkonde commented Sep 17, 2026

Copy link
Copy Markdown

What does this PR do?

Honor textDocument.completion.completionItem.snippetSupport. When it is missing or false, generated completions use schema defaults directly and omit tab stops and snippet escaping. Snippet-capable clients retain the existing placeholders and choices. Literal dollar signs in plain-text defaults are preserved.

The capability is exposed in the public type and LATEST constant. Schema-provided defaultSnippets are offered only to snippet-capable clients, rather than attempting to interpret arbitrary snippet syntax for other clients. This behavior is documented in the README.

The review follow-up removes vscode-snippet-parser entirely. Plain text is generated at the insertion points instead of converting completed snippets afterward; no new dependency remains.

What issues does this PR fix or reference?

Fixes #161.

Is it tested? How?

Windows, Node 24.15.0:

  • All 14 capability regressions pass, including omitted/false/true support, defaults, literal dollar signs, nested required properties, and capability-gated schema snippets.
  • Full test run: 1,356 passing, one pending, two external-schema validation timeouts. A targeted run with the original pre-PR completion source reproduced the drone-schema timeout; the other timeout passed on that baseline retry. The full suite is not claimed green.
  • npm run build passes ESLint, TypeScript, CommonJS, UMD and ESM builds. Prettier and git diff --check pass.

The hosted Node/platform matrix has not been run locally. The original unsupported-capability regression evidence remains in the earlier revision history; the current review adds direct plain-text generation coverage.

AI assistance: Codex implemented this review follow-up, ran the stated checks, and drafted this description. No independent human review is claimed.

Assisted-by: Codex/GPT-6
Signed-off-by: Shubham Padkonde <shubhampadkonde12@gmail.com>

@datho7561 datho7561 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like vscode-snippet-parser is unmaintained (no commits in 9 years). Also, it's not published by the VS Code team, but a seemingly by unmaintained project called onivim. As a result, I don't like adding it as a dependency.

I think this approach in general isn't great, since it's removing the tab stops after the fact rather than avoiding putting them in in the first place. I think the implementation could be much simpler if we wrapped each time we add a tab stop with an if/else. It seems like all the tab stops are added in getInsertTextForGuessedValue or getInsertTextForProperty. Also, we have the helper method this.evaluateTab1Symbol available that can be used to substitute the tab stop with the default value assigned for the tab stop if we want to go the route of pre-evaluating each tab stop.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Server returns snippets even though client doesn't support them

2 participants