-
Notifications
You must be signed in to change notification settings - Fork 226
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
Add snippet support for component completion with EditorRequired attributes #12325
Conversation
…ibutes Co-authored-by: davidwengier <754264+davidwengier@users.noreply.github.com>
davidwengier
left a comment
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
src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Completion/TagHelperCompletionProvider.cs
Outdated
Show resolved
Hide resolved
src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Completion/TagHelperCompletionProvider.cs
Outdated
Show resolved
Hide resolved
src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Completion/TagHelperCompletionProvider.cs
Show resolved
Hide resolved
… snippet as separate completion item Co-authored-by: davidwengier <754264+davidwengier@users.noreply.github.com>
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. |
...isualStudioCode.RazorExtension.Test/Endpoints/Shared/CohostDocumentCompletionEndpointTest.cs
Outdated
Show resolved
Hide resolved
src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Completion/TagHelperCompletionProvider.cs
Outdated
Show resolved
Hide resolved
src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Completion/TagHelperCompletionProvider.cs
Outdated
Show resolved
Hide resolved
Co-authored-by: davidwengier <754264+davidwengier@users.noreply.github.com>
|
@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 <754264+davidwengier@users.noreply.github.com>
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:
|
Co-authored-by: davidwengier <754264+davidwengier@users.noreply.github.com>
src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Completion/TagHelperCompletionProvider.cs
Outdated
Show resolved
Hide resolved
src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Completion/TagHelperCompletionProvider.cs
Outdated
Show resolved
Hide resolved
Co-authored-by: davidwengier <754264+davidwengier@users.noreply.github.com>
|
@copilot please merge main and fix any conflicts or build errors |
Done in commit 1350ef3. Merged main into the branch and resolved conflicts in CohostDocumentCompletionEndpointTest.cs. Both the ComponentWithEditorRequiredAttributes test and the new Blazor data attribute tests from main now coexist. All tests pass. |
| <value>Extract to {0}.css</value> | ||
| </data> | ||
| <data name="ComponentCompletionWithRequiredAttributesLabel" xml:space="preserve"> | ||
| <value>{0} (and req'd attributes...)</value> |
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 add a <comment> element under this that says The term "req'd" is an abbreviation for "required"
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. Added comment element explaining that "req'd" is an abbreviation for "required" to help with localization.
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.
Why use an abbreviation in user visible text?
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 just thought the full word pushed things out to be too wide. Happy to follow up if there is better wording here (but selfishly, I want to get this merged so copilot stops complaining about firewall rules in other PRs 😛)
Co-authored-by: davidwengier <754264+davidwengier@users.noreply.github.com>
src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Completion/TagHelperCompletionProvider.cs
Outdated
Show resolved
Hide resolved
| <value>Extract to {0}.css</value> | ||
| </data> | ||
| <data name="ComponentCompletionWithRequiredAttributesLabel" xml:space="preserve"> | ||
| <value>{0} (and req'd attributes...)</value> |
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.
Why use an abbreviation in user visible text?
…on/TagHelperCompletionProvider.cs
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.