Semantic Snippets - Fix how snippets appear in the completion list#63277
Semantic Snippets - Fix how snippets appear in the completion list#63277akhera99 merged 7 commits intodotnet:mainfrom
Conversation
|
Could you please add a gif/screenshot for the change? Thanks! |
| public override string SnippetIdentifier => "if"; | ||
|
|
||
| public override string SnippetDisplayName => FeaturesResources.Insert_an_if_statement; | ||
| public override string SnippetDescription => FeaturesResources.if_statement; |
There was a problem hiding this comment.
fwiw, I think it's OK to use "Insert an if-statement" as inline description.
There was a problem hiding this comment.
I changed it so it would align with what VS Code says. There is an open question on whether or not the snippets will be discoverable/users will know what they do, so that extra description may help.
@mikadumont what do you think?
| position: position, | ||
| snippetIdentifier: snippetData.SnippetIdentifier, | ||
| glyph: Glyph.Snippet, | ||
| inlineDescription: snippetData.Description, |
There was a problem hiding this comment.
Unrelated to this PR, but I think we should provide description for snippets (tooltips, not inline description)
| Count++; | ||
| // Matching items should be rare, no need to use object pool for this. | ||
| _displayNameToItemsMap[entireDisplayText] = new List<CompletionItem>() { sameNamedItem, item }; | ||
| _displayNameToItemsMap[entireDisplayText] = new List<CompletionItem>() { item, sameNamedItem }; |
There was a problem hiding this comment.
We probabaly don't want to rely on the implementation detail like this to determine the item order. Given the order is ultimatedly decided by SortText, consider using it to differentiate two items instead.
| public class CSharpConsoleSnippetCompletionProviderTests : AbstractCSharpSnippetCompletionProviderTests | ||
| { | ||
| protected override string ItemToCommit => FeaturesResources.Write_to_the_console; | ||
| protected override string ItemToCommit => "cw"; |
There was a problem hiding this comment.
nit: consider making this a constant in the snippet provider and referece it in the tests.
|
Done with the pass. Since this change would affect the aggregated completion list (like sorting, selection, etc.), we need to add end-to-end tests for validation. |
| displayText: displayText, | ||
| displayTextSuffix: displayTextSuffix, | ||
| glyph: glyph, | ||
| sortText: snippetIdentifier + " ", |
Changed the description of the completion items to match that of VS Code.
Since the old snippets and new snippets now share the same display text, we show both but default to the old snippets to appear first in the list.
2022-08-09.13-08-35_Trim.mp4