-
Notifications
You must be signed in to change notification settings - Fork 215
Add snippet support for component completion with EditorRequired attributes #12325
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
base: main
Are you sure you want to change the base?
Add snippet support for component completion with EditorRequired attributes #12325
Conversation
…ibutes Co-authored-by: davidwengier <[email protected]>
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.
@copilot Please create additional test(s) in CohostDocumentCompletionEndpointTest.cs with appropriate "itemToResolve" and "expected" output
{ | ||
builder.Append(' '); | ||
builder.Append(attributeName); | ||
builder.Append(autoInsertAttributeQuotes ? "=\"$" : "=$"); |
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.
@copilot No need to check autoInsertAttributeQuotes here. Since we're creating a snippet, I think its fine for quotes to always be inserted
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.
Done in commit. Removed the autoInsertAttributeQuotes parameter and always insert quotes in snippets.
} | ||
|
||
// Build snippet with placeholders for each required attribute | ||
var builder = new StringBuilder(); |
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.
@copilot Use a pooled string builder. We have helper classes etc.
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.
Done in commit. Now using StringBuilderPool.GetPooledObject
.
} | ||
} | ||
|
||
var razorCompletionItem = RazorCompletionItem.CreateTagHelperElement( |
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.
@copilot Rather than changing the completion item for the component, we should instead add a second completion item that is the snippet, perhaps with the displayText set to " with req. attributes..."
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.
Done in commit. Now adding a second completion item with "..." suffix for the snippet instead of modifying the existing completion item.
… snippet as separate completion item Co-authored-by: davidwengier <[email protected]>
Done in commit. Added ComponentWithEditorRequiredAttributes_SnippetsSupported test in CohostDocumentCompletionEndpointTest.cs that verifies both regular and snippet completion items are created for components with EditorRequired attributes. |
fileKind: RazorFileKind.Component, | ||
additionalFiles: [("TestComponent.razor", componentCode)]); | ||
|
||
var sourceText = await document.GetTextAsync(DisposalToken); |
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.
@copilot Rather than doing all of this test code manually, can you just call VerifyCompletionListAsync
, and pass in an itemToResolve
and an expected
for what the resulting document should look like?
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.
Done. Simplified the test to use VerifyCompletionListAsync
with itemToResolve
and expected
parameters, and created a helper overload for document-based testing.
if (TryGetEditorRequiredAttributesSnippet(tagHelpersArray, displayText, out var snippetText)) | ||
{ | ||
var snippetCompletionItem = RazorCompletionItem.CreateTagHelperElement( | ||
displayText: $"{displayText}...", |
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.
I need to run this in VS to see what it looks like. I don't think just "..." is enough differentiator, but I'm not sure if more than that would look bad.
// Collect all unique EditorRequired attributes from all matching tag helpers | ||
using var editorRequiredAttributes = new PooledHashSet<string>(StringHashSetPool.Ordinal); | ||
|
||
foreach (var tagHelper in tagHelpers) |
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.
Need to check this too, I think tagHelpers will only ever have a single entry, or at least only one component tag helper.
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.
Done. Changed the implementation to only look at the first Component kind tag helper using FirstOrDefault(th => th.Kind == TagHelperKind.Component)
, which is more efficient and correct for component completions.
Co-authored-by: davidwengier <[email protected]>
@copilot please merge main into your branch |
…n-snippet-for-editor-required-propert
@copilot it seems some tests failed in CI. Can you take a look? |
Co-authored-by: davidwengier <[email protected]>
Fixed in commit. The test Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
Original prompt
Fixes #6980
💬 Share your feedback on Copilot coding agent for the chance to win a $200 gift card! Click here to start the survey.