-
Notifications
You must be signed in to change notification settings - Fork 4.2k
Tweaks to existing "copilot completions" processing code to get it ready for auto-fixing copilot suggestions. #79061
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
Conversation
|
|
||
| var addImportOptions = await document.GetAddImportOptionsAsync(searchOptions, cancellationToken).ConfigureAwait(false); | ||
| var addImportOptions = await document.GetAddImportOptionsAsync( | ||
| searchOptions, cleanupDocument: true, cancellationToken).ConfigureAwait(false); |
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.
added a flag as copilot-fixup wants to add imports without that making other undesired changes to other parts of the document. (specifically, case correcting things in VB).
| featureId, accepted, proposalId, analysisResult, cancellationToken).Dispose(); | ||
| } | ||
|
|
||
| private static ImmutableArray<TextChange> Normalize(IEnumerable<TextChange> textChanges) |
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.
moved to shared utilites outside of 'change analysis. needed by auto-fix-copilot-changes work.
| var proposalId = proposal.ProposalId; | ||
|
|
||
| var solution = CopilotEditorUtilities.TryGetAffectedSolution(proposal); | ||
| if (solution is null) |
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.
standard helper to make sure this is even a proposal we can consider fixing. will be used by new feature as well.
|
|
||
| // We're about to potentially make multiple calls to oop here. So keep a session alive to avoid | ||
| // resyncing any data unnecessary. | ||
| using var _1 = await RemoteKeepAliveSession.CreateAsync(solution, cancellationToken).ConfigureAwait(false); |
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.
somethign i noticed while doing work here. can help perf if copilot is suggesting changes to multiple documents.
|
|
||
| internal static class CopilotEditorUtilities | ||
| { | ||
| public static Solution? TryGetAffectedSolution(ProposalBase proposal) |
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.
Ensures the edit is touching only files we can properly analyze within a single roslyn solution snapshot.
|
merging, integration test failures are expected on main-vs-deps. |
Extracted from #78966 to make that PR easier to review.