Skip to content

Support defaultSnippets - #111

Open
Ahmedmhmud wants to merge 7 commits into
hyperjump-io:mainfrom
Ahmedmhmud:feat/support-defaultSnippets
Open

Ahmedmhmud wants to merge 7 commits into
hyperjump-io:mainfrom
Ahmedmhmud:feat/support-defaultSnippets

Conversation

@Ahmedmhmud

Copy link
Copy Markdown

Summary

This PR adds support for VS Code custom keyword defaultSnippets.

What changed

  • Added completion handling for defaultSnippets annotations.
  • Supported snippet insertion for the main defaultSnippets forms:
    • bodyText
    • body as a string
    • body as an array of lines

The VS Code keyword can provide the snippet body in multiple forms so the completion logic now handles each supported variant.

Tests

  • Added tests covering these cases to verify the completion behavior.

Closes #107

@jdesrosiers jdesrosiers left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I think the AnnotationEvaluationPlugin changes aren't going to be sufficient. See my comment below. Start by creating a test that illustrates the problem.

Please create a new file for these tests. I think this is a distinct feature and that's a good excuse to not pile onto a file that's already several thousand lines long.

Comment thread language-server/src/features/completions/Completions.ts Outdated
Comment thread language-server/src/features/completions/ValueCompletionsProvider.ts Outdated
Comment thread language-server/src/features/completions/ValueCompletionsProvider.ts Outdated
Comment thread language-server/src/features/completions/ValueCompletionsProvider.ts Outdated
Comment thread language-server/src/vscode-vocabulary.ts
Comment thread language-server/src/features/AnnotationsEvaluationPlugin.ts Outdated

@jdesrosiers jdesrosiers left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I just realized that this should be its own CompletionProvider instead of adding to ValueCompletionProvider. I originally assumed it would be modifying the existing completions somehow, but that's not what's happening. It's just adding additional completions. So, there's no reason for them to be coupled.

@Ahmedmhmud
Ahmedmhmud force-pushed the feat/support-defaultSnippets branch from 390abb7 to 5c6fa17 Compare September 24, 2026 16:41
@Ahmedmhmud

Copy link
Copy Markdown
Author

Changes made:

  • Add beforeKeyword traversal to AnnotationsEvaluationPlugin to collect annotations for incomplete locations (properties, patternProperties, additionalProperties, items, prefixItems), matching CompletionsEvaluationPlugin's buildCompletions().
  • Fix a plugin registration collision where Hover, DocumentColors, and Completions each registered their own AnnotationsEvaluationPlugin instance under the same id, silently overwriting one another.
  • Add DefaultSnippetsCompletionsProvider as its own CompletionsProvider, decoupled from ValueCompletionsProvider.
  • Define for CompletionsEvaluationPlugin it's own id the way AnnotationsEvaluationPlugin does.
  • Serialize snippet body/bodyText to match VS Code's behavior: bodyText inserted as-is, body JSON-stringified.
  • Add test coverage in a new DefaultSnippetsCompletionsProvider.test.ts.

@jdesrosiers jdesrosiers left a comment •

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I push some clean up and fixed a bug that I noticed when fixing some positions in the tests.

I notice at this point that there there's a significant amount of duplication in the CompletionProviders. See if you can refactor to improve that situation. I think much of the duplication can move to Completions.

And please rebase as well.

@Ahmedmhmud

Ahmedmhmud commented Sep 26, 2026 •

Copy link
Copy Markdown
Author

I agree with you and I have some ideas for this one.
On it

@jdesrosiers

Copy link
Copy Markdown
Collaborator

The tests are passing for me locally, so it looks like we have a race condition bug again. 😢

@Ahmedmhmud

Copy link
Copy Markdown
Author

No problem, I will track what went wrong

@Ahmedmhmud

Copy link
Copy Markdown
Author

Hi @jdesrosiers
I refactored as we agreed, but there is a test that is failing specifically the last test in ValueCompletionsProvider.test.ts
This one is probably failing because of the last merged PR.
I tried to track the error and it was in validateSchema() in JsonDocument.ts and put a try-catch block but other 7 tests failed.

So I will study the changes in the last PR and try to find why is this one failing, and I will push the newest version of this current feature for your review.

@Ahmedmhmud
Ahmedmhmud force-pushed the feat/support-defaultSnippets branch from b1439ea to 42af79b Compare September 27, 2026 01:32
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.

Support defaultSnippets VSCode's custom keyword

2 participants