-
Notifications
You must be signed in to change notification settings - Fork 4k
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
Semantic snippets: Implement do
snippet
#72842
Conversation
...CSharpTest/Completion/CompletionProviders/Snippets/CSharpDoSnippetCompletionProviderTests.cs
Outdated
Show resolved
Hide resolved
src/Features/CSharp/Portable/Completion/CompletionProviders/SnippetCompletionProvider.cs
Outdated
Show resolved
Hide resolved
src/Features/CSharp/Portable/Snippets/CSharpDoWhileLoopStatementProvider.cs
Outdated
Show resolved
Hide resolved
src/Features/CSharp/Portable/Snippets/CSharpDoWhileLoopStatementProvider.cs
Outdated
Show resolved
Hide resolved
Co-authored-by: Cyrus Najmabadi <[email protected]>
@CyrusNajmabadi Regardless your request on extracting constants: I tried to preserve layering and separated identifiers into 2 categories: those which can theoretically be reused in VB (e.g. |
@CyrusNajmabadi PTAL |
|
||
protected override SyntaxNode GetCondition(SyntaxNode node) | ||
{ | ||
var doStatement = (DoStatementSyntax)node; |
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.
nit: int eh future, it would nice to use generics (with teh base type) to make it so that all this is strongly typed.
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 particular example this would just move the hard cast one layer above:
Lines 21 to 26 in e6c6877
protected override ImmutableArray<SnippetPlaceholder> GetPlaceHolderLocationsList(SyntaxNode node, ISyntaxFacts syntaxFacts, CancellationToken cancellationToken) | |
{ | |
if (ConstructedFromInlineExpression) | |
return []; | |
var condition = GetCondition(node); |
GetPlaceHolderLocationsList
is from the AbstractSnippetProvider
class, which is a base class for all snippets. And even if we try to make AbstractSnippetProvider
generic as well, we will still need to hard cast here (see how mainChangeNode
is just found via annotation, the cast would be here):roslyn/src/Features/Core/Portable/Snippets/SnippetProviders/AbstractSnippetProvider.cs
Lines 110 to 123 in e6c6877
var mainChangeNode = reformattedRoot.GetAnnotatedNodes(FindSnippetAnnotation).FirstOrDefault(); | |
Contract.ThrowIfNull(caretTarget); | |
Contract.ThrowIfNull(mainChangeNode); | |
var annotatedReformattedDocument = documentWithIndentation.WithSyntaxRoot(reformattedRoot); | |
// All the TextChanges from the original document. Will include any imports (if necessary) and all snippet associated | |
// changes after having been formatted. | |
var changes = await annotatedReformattedDocument.GetTextChangesAsync(document, cancellationToken).ConfigureAwait(false); | |
// Gets a listing of the identifiers that need to be found in the snippet TextChange | |
// and their associated TextSpan so they can later be converted into an LSP snippet format. | |
var placeholders = GetPlaceHolderLocationsList(mainChangeNode, syntaxFacts, cancellationToken); |
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.
Yup. mkaing these changes in #72892
Thanks! |
Part of #64144 (don't forget to check the item when merging)