Skip to content
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

Additional incremental tweaks #4143

Merged
merged 2 commits into from
Oct 23, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,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))
* Do not send file changed events for .cs files (PR: [#4141](https://github.com/OmniSharp/omnisharp-vscode/pull/4141))
* Do not send file changed events for .cs files (PR: [#4141](https://github.com/OmniSharp/omnisharp-vscode/pull/4141), [#4143](https://github.com/OmniSharp/omnisharp-vscode/pull/4143))
* Update Razor to 6.0.0-alpha1.20521.3:
* Improvements to HTML colorization for non-C# portions of the document.
* Bug fix - the `razor.format.enable` option is honored again
Expand Down
25 changes: 16 additions & 9 deletions src/features/changeForwarding.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,17 +49,24 @@ function forwardFileChanges(server: OmniSharpServer): IDisposable {
return;
}

if (changeType === FileChangeType.Change && uri.fsPath.endsWith(".cs")) {
// When a file changes on disk a FileSystemEvent is generated as well as
// a DidChangeTextDocumentEvent. The OmniSharp server listens for Change events
// for ".cs" files and reloads their text from disk. This creates a situation where the server
// may have updated the document to reflect disk and also recieves a set of TextChanges
// to apply to the document. In order to avoid that situation, we will not send Change events
// for ".cs" files and instead allow them to be updated via the DidChangeTextDocumentEvent.
const docs = workspace.textDocuments.filter(doc => doc.uri.fsPath === uri.fsPath);
if (Array.isArray(docs) && docs.some(doc => !doc.isClosed)) {
// When a file changes on disk a FileSystemEvent is generated as well as a
// DidChangeTextDocumentEvent.The ordering of these is:
// 1. This method is called back. vscode's TextDocument has not yet been reloaded, so it has
// the version from before the changes are applied.
// 2. vscode reloads the file, and fires onDidChangeTextDocument. The document has been updated,
// and the changes have the delta.
// If we send this change to the server, then it will reload from the disk, which means it will
// be synchronized to the version after the changes. Then, onDidChangeTextDocument will fire and
// send the delta changes, which will cause the server to apply those exact changes. The results
// being that the file is now in an inconsistent state.
// If the document is closed, however, it will no longer be synchronized, so the text change will
// not be triggered and we should tell the server to reread from the disk.
return;
}

let req = { FileName: uri.fsPath, changeType };
const req = { FileName: uri.fsPath, changeType };

serverUtils.filesChanged(server, [req]).catch(err => {
console.warn(`[o] failed to forward file change event for ${uri.fsPath}`, err);
Expand All @@ -75,7 +82,7 @@ function forwardFileChanges(server: OmniSharpServer): IDisposable {
}

if (changeType === FileChangeType.Delete) {
let requests = [{ FileName: uri.fsPath, changeType: FileChangeType.DirectoryDelete }];
const requests = [{ FileName: uri.fsPath, changeType: FileChangeType.DirectoryDelete }];

serverUtils.filesChanged(server, requests).catch(err => {
console.warn(`[o] failed to forward file change event for ${uri.fsPath}`, err);
Expand Down