-
Notifications
You must be signed in to change notification settings - Fork 676
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
Don't apply edits on server #4133
Conversation
The code action provider and rename provider were expecting the server to apply the file changes, which then messes up incremental buffer changes. This sends for false for these parameters, and refactors the handling of code action responses to do the renames locally. As part of that, I also refactored the protocol to accurately reflect O#'s hierarchy, and share the edit building between the fixAllProvider and the runCodeAction provider. Fixes dotnet#4128.
} | ||
} | ||
|
||
for (const change of changes) { |
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.
This is the actual after change here.
let fileToOpen: Uri = null; | ||
let renamedFiles: Uri[] = []; | ||
|
||
for (let change of response.Changes) { |
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.
The after change replaced this logic.
} | ||
|
||
for (let change of response.Changes) { | ||
if (change.ModificationType == FileModificationType.Opened) |
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.
This if
is unchanged.
if (change.ModificationType == FileModificationType.Modified) | ||
{ | ||
let uri = vscode.Uri.file(change.FileName); | ||
if (renamedFiles.some(r => r == uri)) |
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.
This logic is removed.
} | ||
|
||
let edits: vscode.TextEdit[] = []; | ||
for (let textChange of change.Changes) { |
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.
This logic is unchanged, other than using const
where appropriate and correcting for the necessary cast on change
to the correct type.
// If files were renamed that weren't the active editor, their tabs will | ||
// be left open and marked as "deleted" by VS Code | ||
let next = applyEditPromise; | ||
if (renamedFiles.some(r => r.fsPath == vscode.window.activeTextEditor.document.uri.fsPath)) |
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.
This logic is removed and unneeded, as we now do the rename ourselves.
Codecov Report
@@ Coverage Diff @@
## master #4133 +/- ##
=======================================
Coverage 85.99% 85.99%
=======================================
Files 60 60
Lines 1857 1857
Branches 215 215
=======================================
Hits 1597 1597
Misses 200 200
Partials 60 60
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
The code action provider and rename provider were expecting the server to apply the file changes, which then messes up incremental buffer changes. This sends for false for these parameters, and refactors the handling of code action responses to do the renames locally. As part of that, I also refactored the protocol to accurately reflect O#'s hierarchy, and share the edit building between the fixAllProvider and the runCodeAction provider. Fixes #4128.