Skip to content

Commit

Permalink
Merge pull request #4133 from 333fred/rename-not-apply
Browse files Browse the repository at this point in the history 
Don't apply edits on server
  • Loading branch information
@JoeRobich
JoeRobich authored Oct 22, 2020
2 parents f97ba36 + 1ca331b commit dde0eaf
Show file tree
Hide file tree
Showing 6 changed files with 95 additions and 151 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@

## 1.23.5 (Not yet released)
* Set meaning of UseGlobalMono "auto" to "never" since Mono 6.12.0 still ships with MSBuild 16.7 (PR: [#4130](https://github.com/OmniSharp/omnisharp-vscode/pull/4130))
* Ensure that the rename identifier and run code action providers do not apply changes twice PR: [4133](https://github.com/OmniSharp/omnisharp-vscode/pull/4133)

## 1.23.4 (October, 19, 2020)
* Use incremental changes to update language server (PR: [#4088](https://github.com/OmniSharp/omnisharp-vscode/pull/4088))
Expand Down
87 changes: 9 additions & 78 deletions src/features/codeActionProvider.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,11 @@ import * as vscode from 'vscode';
import { OmniSharpServer } from '../omnisharp/server';
import AbstractProvider from './abstractProvider';
import * as protocol from '../omnisharp/protocol';
import { toRange2 } from '../omnisharp/typeConversion';
import * as serverUtils from '../omnisharp/utils';
import { FileModificationType } from '../omnisharp/protocol';
import { Uri } from 'vscode';
import CompositeDisposable from '../CompositeDisposable';
import OptionProvider from '../observers/OptionProvider';
import { LanguageMiddlewareFeature } from '../omnisharp/LanguageMiddlewareFeature';
import { buildEditForResponse } from '../omnisharp/fileOperationsResponseEditBuilder';

export default class CodeActionProvider extends AbstractProvider implements vscode.CodeActionProvider {

Expand Down Expand Up @@ -84,7 +82,8 @@ export default class CodeActionProvider extends AbstractProvider implements vsco
Selection: selection,
Identifier: codeAction.Identifier,
WantsTextChanges: true,
WantsAllCodeActionOperations: true
WantsAllCodeActionOperations: true,
ApplyTextChanges: false
};

return {
Expand All @@ -101,80 +100,12 @@ export default class CodeActionProvider extends AbstractProvider implements vsco

private async _runCodeAction(req: protocol.V2.RunCodeActionRequest, token: vscode.CancellationToken): Promise<boolean | string | {}> {

return serverUtils.runCodeAction(this._server, req).then(async response => {
if (response && Array.isArray(response.Changes)) {

let edit = new vscode.WorkspaceEdit();

let fileToOpen: Uri = null;
let renamedFiles: Uri[] = [];

for (let change of response.Changes) {
if (change.ModificationType == FileModificationType.Renamed)
{
// The file was renamed. Omnisharp has already persisted
// the right changes to disk. We don't need to try to
// apply text changes (and will skip this file if we see an edit)
renamedFiles.push(Uri.file(change.FileName));
}
}

for (let change of response.Changes) {
if (change.ModificationType == FileModificationType.Opened)
{
// The CodeAction requested that we open a file.
// Record that file name and keep processing CodeActions.
// If a CodeAction requests that we open multiple files
// we only open the last one (what would it mean to open multiple files?)
fileToOpen = vscode.Uri.file(change.FileName);
}

if (change.ModificationType == FileModificationType.Modified)
{
let uri = vscode.Uri.file(change.FileName);
if (renamedFiles.some(r => r == uri))
{
// This file got renamed. OmniSharp has already
// persisted the new file with any applicable changes.
continue;
}

let edits: vscode.TextEdit[] = [];
for (let textChange of change.Changes) {
edits.push(vscode.TextEdit.replace(toRange2(textChange), textChange.NewText));
}

edit.set(uri, edits);
}
}

// Allow language middlewares to re-map its edits if necessary.
edit = await this._languageMiddlewareFeature.remap("remapWorkspaceEdit", edit, token);

let applyEditPromise = vscode.workspace.applyEdit(edit);

// Unfortunately, the textEditor.Close() API has been deprecated
// and replaced with a command that can only close the active editor.
// 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))
{
next = applyEditPromise.then(_ =>
{
return vscode.commands.executeCommand("workbench.action.closeActiveEditor");
});
}

return fileToOpen != null
? next.then(_ =>
{
return vscode.commands.executeCommand("vscode.open", fileToOpen);
})
: next;
}
}, async (error) => {
return serverUtils.runCodeAction(this._server, req).then(response => {
if (response) {
return buildEditForResponse(response.Changes, this._languageMiddlewareFeature, token);
}
}, async (error) => {
return Promise.reject(`Problem invoking 'RunCodeAction' on OmniSharp server: ${error}`);
});
}
}
}
74 changes: 5 additions & 69 deletions src/features/fixAllProvider.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,12 @@ import * as vscode from 'vscode';
import * as serverUtils from '../omnisharp/utils';
import * as protocol from '../omnisharp/protocol';
import { OmniSharpServer } from '../omnisharp/server';
import { FixAllScope, FixAllItem, FileModificationType } from '../omnisharp/protocol';
import { Uri } from 'vscode';
import { FixAllScope, FixAllItem } from '../omnisharp/protocol';
import CompositeDisposable from '../CompositeDisposable';
import AbstractProvider from './abstractProvider';
import { toRange2 } from '../omnisharp/typeConversion';
import { LanguageMiddlewareFeature } from '../omnisharp/LanguageMiddlewareFeature';
import { buildEditForResponse } from '../omnisharp/fileOperationsResponseEditBuilder';
import { CancellationToken } from 'vscode-languageserver-protocol';

export class FixAllProvider extends AbstractProvider implements vscode.CodeActionProvider {
public constructor(private server: OmniSharpServer, languageMiddlewareFeature: LanguageMiddlewareFeature) {
Expand Down Expand Up @@ -80,72 +80,8 @@ export class FixAllProvider extends AbstractProvider implements vscode.CodeActio
ApplyChanges: false
});

if (response && Array.isArray(response.Changes)) {
let edit = new vscode.WorkspaceEdit();

let fileToOpen: Uri = null;
let renamedFiles: Uri[] = [];

for (let change of response.Changes) {
if (change.ModificationType == FileModificationType.Renamed)
{
// The file was renamed. Omnisharp has already persisted
// the right changes to disk. We don't need to try to
// apply text changes (and will skip this file if we see an edit)
renamedFiles.push(Uri.file(change.FileName));
}
}

for (let change of response.Changes) {
if (change.ModificationType == FileModificationType.Opened)
{
// The CodeAction requested that we open a file.
// Record that file name and keep processing CodeActions.
// If a CodeAction requests that we open multiple files
// we only open the last one (what would it mean to open multiple files?)
fileToOpen = vscode.Uri.file(change.FileName);
}

if (change.ModificationType == FileModificationType.Modified)
{
let uri = vscode.Uri.file(change.FileName);
if (renamedFiles.some(r => r == uri))
{
// This file got renamed. Omnisharp has already
// persisted the new file with any applicable changes.
continue;
}

let edits: vscode.TextEdit[] = [];
for (let textChange of change.Changes) {
edits.push(vscode.TextEdit.replace(toRange2(textChange), textChange.NewText));
}

edit.set(uri, edits);
}
}

let applyEditPromise = vscode.workspace.applyEdit(edit);

// Unfortunately, the textEditor.Close() API has been deprecated
// and replaced with a command that can only close the active editor.
// 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))
{
next = applyEditPromise.then(_ =>
{
return vscode.commands.executeCommand("workbench.action.closeActiveEditor");
});
}

return fileToOpen != null
? next.then(_ =>
{
return vscode.commands.executeCommand("vscode.open", fileToOpen);
})
: next;
if (response) {
return buildEditForResponse(response.Changes, this._languageMiddlewareFeature, CancellationToken.None);
}
}
}
1 change: 1 addition & 0 deletions src/features/renameProvider.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ export default class OmnisharpRenameProvider extends AbstractSupport implements
let req = createRequest<protocol.RenameRequest>(document, position);
req.WantsTextChanges = true;
req.RenameTo = newName;
req.ApplyTextChanges = false;

try {
let response = await serverUtils.rename(this._server, req, token);
Expand Down
63 changes: 63 additions & 0 deletions src/omnisharp/fileOperationsResponseEditBuilder.ts
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
/*---------------------------------------------------------------------------------------------
* Copyright (c) Microsoft Corporation. All rights reserved.
* Licensed under the MIT License. See License.txt in the project root for license information.
*--------------------------------------------------------------------------------------------*/

import * as vscode from 'vscode';
import { Uri } from 'vscode';
import { LanguageMiddlewareFeature } from './LanguageMiddlewareFeature';
import { FileModificationType, FileOperationResponse, ModifiedFileResponse, RenamedFileResponse } from "./protocol";
import { toRange2 } from './typeConversion';

export async function buildEditForResponse(changes: FileOperationResponse[], languageMiddlewareFeature: LanguageMiddlewareFeature, token: vscode.CancellationToken): Promise<boolean> {
let edit = new vscode.WorkspaceEdit();

let fileToOpen: Uri = null;

if (!changes || !Array.isArray(changes) || !changes.length) {
return true;
}

for (const change of changes) {
if (change.ModificationType == FileModificationType.Opened) {
// The CodeAction requested that we open a file.
// Record that file name and keep processing CodeActions.
// If a CodeAction requests that we open multiple files
// we only open the last one (what would it mean to open multiple files?)
fileToOpen = vscode.Uri.file(change.FileName);
}

if (change.ModificationType == FileModificationType.Modified) {
const modifiedChange = <ModifiedFileResponse>change;
const uri = vscode.Uri.file(modifiedChange.FileName);
let edits: vscode.TextEdit[] = [];
for (let textChange of modifiedChange.Changes) {
edits.push(vscode.TextEdit.replace(toRange2(textChange), textChange.NewText));
}

edit.set(uri, edits);
}
}

for (const change of changes) {
if (change.ModificationType == FileModificationType.Renamed) {
const renamedChange = <RenamedFileResponse>change;
edit.renameFile(vscode.Uri.file(renamedChange.FileName), vscode.Uri.file(renamedChange.NewFileName));
}
}

// Allow language middlewares to re-map its edits if necessary.
edit = await languageMiddlewareFeature.remap("remapWorkspaceEdit", edit, token);

const applyEditPromise = vscode.workspace.applyEdit(edit);

// Unfortunately, the textEditor.Close() API has been deprecated
// and replaced with a command that can only close the active editor.
// If files were renamed that weren't the active editor, their tabs will
// be left open and marked as "deleted" by VS Code
return fileToOpen != null
? applyEditPromise.then(_ => {
return vscode.commands.executeCommand("vscode.open", fileToOpen);
})
: applyEditPromise;
}
20 changes: 16 additions & 4 deletions src/omnisharp/protocol.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -262,7 +262,7 @@ export interface GetCodeActionsResponse {

export interface RunFixAllActionResponse {
Text: string;
Changes: ModifiedFileResponse[];
Changes: FileOperationResponse[];
}

export interface FixAllItem {
Expand Down Expand Up @@ -371,13 +371,24 @@ export interface DotNetFramework {
export interface RenameRequest extends Request {
RenameTo: string;
WantsTextChanges?: boolean;
ApplyTextChanges: boolean;
}

export interface ModifiedFileResponse {
export interface FileOperationResponse {
FileName: string;
ModificationType: FileModificationType;
}

export interface ModifiedFileResponse extends FileOperationResponse {
Buffer: string;
Changes: TextChange[];
ModificationType: FileModificationType;
}

export interface RenamedFileResponse extends FileOperationResponse {
NewFileName: string;
}

export interface OpenFileResponse extends FileOperationResponse {
}

export enum FileModificationType {
Expand Down Expand Up @@ -596,10 +607,11 @@ export namespace V2 {
Selection?: Range;
WantsTextChanges: boolean;
WantsAllCodeActionOperations: boolean;
ApplyTextChanges: boolean;
}

export interface RunCodeActionResponse {
Changes: ModifiedFileResponse[];
Changes: FileOperationResponse[];
}

export interface MSBuildProjectDiagnostics {
Expand Down

0 comments on commit dde0eaf

Please sign in to comment.