Skip to content
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

Provide the ability for an extension to conditionally provide snippets that render the same as package.json-provided snippets #41202

Closed
DanTup opened this issue Jan 5, 2018 · 11 comments
Assignees
Labels
snippets under-discussion Issue is under discussion for relevance, priority, approach

Comments

@DanTup
Copy link
Contributor

DanTup commented Jan 5, 2018

I'd like to be able to conditionally provide snippets based on some knowledge of the project type a user has opened with my extension. I previously opened #26943 requested when support for snippets but it was suggested that I just change to a CompletionItemProvider instead.

I've implemented a provider that reads the standard snippet JSON format in an attempt to provide the exact same behaviour as when the snippets were contributed via package.json:

export class SnippetCompletionItemProvider implements CompletionItemProvider {
	private completions = new CompletionList();

	constructor(filename: string) {
		const snippets = require(path.join("../../..", filename));
		for (const snippetType of Object.keys(snippets)) {
			for (const snippetName of Object.keys(snippets[snippetType])) {
				const snippet = snippets[snippetType][snippetName];
				const completionItem = new CompletionItem(snippetName, CompletionItemKind.Snippet);
				completionItem.filterText = snippet.prefix;
				completionItem.insertText = new SnippetString(
					isArray(snippet.body)
						? snippet.body.join("\n")
						: snippet.body,
				);
				completionItem.detail = snippet.description;
				completionItem.documentation = new MarkdownString().appendCodeblock(completionItem.insertText.value);
				this.completions.items.push(completionItem);
			}
		}
	}

	public provideCompletionItems(
		document: TextDocument, position: Position, token: CancellationToken,
	): CompletionList {
		return this.completions;
	}
}

This appears to work fine and renders snippets that look just like the built-in ones, with one exception - the placeholders are hidden in the built-in snippets but visible in mine:

Code's snippets

My snippets

I've been searching through the code and believe this is because of _rewriteBogousVariables from here.

I could copy that code into my completion provider, but it means I have to maintain it if there are any changes/bugfixes in the future. I think it would be better if Code exposed a way for us to be able to provide snippets that render the same as Code's but that we can be added conditionally. (Alternatively, is it possible #26943 could be re-considered? I think it would be a nicer solution!).

@jrieken
Copy link
Member

jrieken commented Jan 8, 2018

Hm, the different is because of this: this.documentation = new MarkdownString().appendCodeblock('', new SnippetParser().text(this.snippet.codeSnippet));. It take the parser, computes the actual insert text (w/o placeholders) and uses that as doc. That's something the core can do with easy but nothing extension easily can do...

Unsure what to do about it...

@jrieken jrieken added the under-discussion Issue is under discussion for relevance, priority, approach label Jan 8, 2018
@DanTup
Copy link
Contributor Author

DanTup commented Jan 8, 2018

Would you consider implementing #26943 instead? It would avoid having to leak any of this snippet-specific code out into extensions and also be consistent with a lot of the other things in package.json that support when conditions (like commands). It seems like an elegant solution to this problem (it also might be relatively simple to carry the condition into the snippet completion provider and evaluate it there during completion?).

@DanTup
Copy link
Contributor Author

DanTup commented Jan 8, 2018

Actually, I just read your comments about snippets wanting scopes, so maybe pushing people to completion providers is better - so maybe finding a way to expose this does make more sense..

@jrieken
Copy link
Member

jrieken commented Jan 8, 2018

It would avoid having to leak any of this snippet-specific code out into extensions and also be consistent

Another way would be make it the default presentation for a snippet completion. So, when the type is snippet we always generate such a string and append to the doc. Tho it will collide with suggestions that already try to emulate this.

@DanTup
Copy link
Contributor Author

DanTup commented Jan 8, 2018

Ah yeah, I forgot there was a kind specifically for snippets. Though whether users would expect the rendering to change like that based on the kind, I'm not sure.

In the interest of shipping my changes so I can add project-specific snippets, I'm just going to hide the preview for now (not set the documentation property). That way I wouldn't be impacted if there are any changes, it's no worse than snippets were a while back (before the previews).

If you come up with a good way to handle this, I'll update it :-)

@jrieken
Copy link
Member

jrieken commented Jan 8, 2018

If you come up with a good way to handle this, I'll update it :-)

A good idea might be to use the preview only when the documentation is missing ;-)

@DanTup
Copy link
Contributor Author

DanTup commented Jan 10, 2018

That'd work for me; though I don't know if it'd make sense to support both. If I added documentation that was just a line of text, the preview would disappear?

Thinking about the original problem again (my preview has $tokens in it and Code's doesn't), if the preview is always effectively replacing them with a blank value, would just stripping out $xxx with a regex make them the same? It's not a good long-term fix (since if you change the display of snippets we'll fall out of sync) but it's something I can do myself today to improve the experience.

@jrieken
Copy link
Member

jrieken commented Jan 11, 2018

Thinking about the original problem again (my preview has $tokens in it and Code's doesn't), if the preview is always effectively replacing them with a blank value,

That's just variables... Also placeholders come into play, so $1, $2 etc then also all other snippet features...

@DanTup
Copy link
Contributor Author

DanTup commented Jan 11, 2018

@jrieken I think that stuff all already works using my completion provider at the top? The only difference I noticed (but it's possible I didn't test properly!) was the preview being different?

@DanTup
Copy link
Contributor Author

DanTup commented Feb 7, 2018

On reflection I've decided to leave this as-is. It means the user may see things like typedef ${1:Type} ${2:Name}(${3:params}); but that's probably better than seeing just typedef and then all these placeholders appearing when they complete.

I'll revisit if you add a better way to handle it.

@jrieken
Copy link
Member

jrieken commented Feb 27, 2018

Ok. Closing until you'll change your mind.

@jrieken jrieken closed this as completed Feb 27, 2018
@vscodebot vscodebot bot locked and limited conversation to collaborators Apr 13, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
snippets under-discussion Issue is under discussion for relevance, priority, approach
Projects
None yet
Development

No branches or pull requests

2 participants