-
Notifications
You must be signed in to change notification settings - Fork 418
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
Listen for file renames #987
Changes from 14 commits
70f8486
47804cf
2459ea0
1096cd6
c46fad5
2f2ea4c
d5a8477
e9e1fd7
2882a33
c9db400
634be55
4db48c7
9847dc4
2b19763
c7884fe
b3cb182
3ef284e
14e9626
b122830
9802d7e
9796a50
e8652fa
9b96a8d
f305c86
36c145b
5d0b91b
858bcbb
259bc76
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,12 +1,20 @@ | ||
using System; | ||
using OmniSharp.Models.FilesChanged; | ||
|
||
namespace OmniSharp.FileWatching | ||
{ | ||
// TODO: Flesh out this API more | ||
public interface IFileSystemWatcher | ||
{ | ||
void Watch(string path, Action<string> callback); | ||
void Watch(string path, Action<string, FileChangeType> callback); | ||
|
||
void TriggerChange(string path); | ||
/// <summary> | ||
/// Called when a file is created, changed, or deleted. | ||
/// </summary> | ||
/// <param name="path">The path to the file</param> | ||
/// <param name="changeType">The type of change. Hosts are not required to pass a change type</param> | ||
void TriggerChange(string path, FileChangeType changeType); | ||
|
||
void WatchDirectory(string path, Action<string, FileChangeType> callback); | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
namespace OmniSharp.Models.FilesChanged | ||
{ | ||
public enum FileChangeType | ||
{ | ||
Unspecified = 0, | ||
Change, | ||
Create, | ||
Delete | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,8 +1,11 @@ | ||
using System.Collections.Generic; | ||
using OmniSharp.Mef; | ||
using System.Collections.Generic; | ||
|
||
namespace OmniSharp.Models.FilesChanged | ||
{ | ||
[OmniSharpEndpoint(OmniSharpEndpoints.FilesChanged, typeof(IEnumerable<Request>), typeof(FilesChangedResponse))] | ||
public class FilesChangedRequest : IRequest { } | ||
[OmniSharpEndpoint(OmniSharpEndpoints.FilesChanged, typeof(IEnumerable<FilesChangedRequest>), typeof(FilesChangedResponse))] | ||
public class FilesChangedRequest : Request | ||
{ | ||
public FileChangeType ChangeType { get; set; } | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This will serialize as an int (I think, I didn't see any where we configure |
||
} | ||
} |
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,24 +1,37 @@ | ||
using System; | ||
using System.Collections.Generic; | ||
using System.IO; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sort usings so |
||
using OmniSharp.Models.FilesChanged; | ||
|
||
namespace OmniSharp.FileWatching | ||
{ | ||
public class ManualFileSystemWatcher : IFileSystemWatcher | ||
{ | ||
private readonly Dictionary<string, Action<string>> _callbacks = new Dictionary<string, Action<string>>(); | ||
private readonly Dictionary<string, Action<string, FileChangeType>> _callbacks = new Dictionary<string, Action<string, FileChangeType>>(); | ||
private readonly Dictionary<string, Action<string, FileChangeType>> _directoryCallBacks = new Dictionary<string, Action<string, FileChangeType>>(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. One concern I have here is that only a single callback can be called for a particular path. And, the last callback registered will win. I don't think this will be a problem so far, but it may be in the future -- especially since we're watching directories. I can easily imagine some other project system (like script or cake) wanting to listen to the same directories that the MSBuild project system does. Should we combine callbacks using There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That's a good point--will add this |
||
|
||
public void TriggerChange(string path) | ||
public void TriggerChange(string path, FileChangeType changeType) | ||
{ | ||
Action<string> callback; | ||
if (_callbacks.TryGetValue(path, out callback)) | ||
if (_callbacks.TryGetValue(path, out var callback)) | ||
{ | ||
callback(path); | ||
callback(path, changeType); | ||
} | ||
|
||
var directoryPath = Path.GetDirectoryName(path); | ||
if (_directoryCallBacks.TryGetValue(directoryPath, out var fileCallback)) | ||
{ | ||
fileCallback(path, changeType); | ||
} | ||
} | ||
|
||
public void Watch(string path, Action<string> callback) | ||
public void Watch(string path, Action<string, FileChangeType> callback) | ||
{ | ||
_callbacks[path] = callback; | ||
} | ||
|
||
public void WatchDirectory(string path, Action<string, FileChangeType> callback) | ||
{ | ||
_directoryCallBacks[path] = callback; | ||
} | ||
} | ||
} | ||
} |
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,17 +1,12 @@ | ||
using System; | ||
using System.Collections.Generic; | ||
using System.Collections.Immutable; | ||
using System.Composition; | ||
using System.IO; | ||
using System.Linq; | ||
using System.Threading.Tasks; | ||
using Microsoft.CodeAnalysis; | ||
using Microsoft.CodeAnalysis; | ||
using Microsoft.CodeAnalysis.CSharp; | ||
using Microsoft.Extensions.Configuration; | ||
using Microsoft.Extensions.Logging; | ||
using OmniSharp.Eventing; | ||
using OmniSharp.FileWatching; | ||
using OmniSharp.Models.Events; | ||
using OmniSharp.Models.FilesChanged; | ||
using OmniSharp.Models.UpdateBuffer; | ||
using OmniSharp.Models.WorkspaceInformation; | ||
using OmniSharp.MSBuild.Models; | ||
using OmniSharp.MSBuild.Models.Events; | ||
|
@@ -20,6 +15,13 @@ | |
using OmniSharp.MSBuild.SolutionParsing; | ||
using OmniSharp.Options; | ||
using OmniSharp.Services; | ||
using System; | ||
using System.Collections.Generic; | ||
using System.Collections.Immutable; | ||
using System.Composition; | ||
using System.IO; | ||
using System.Linq; | ||
using System.Threading.Tasks; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sort usings with There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I swear my option is resetting underneath me. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. bummer 😞 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe you've discovered a VS bug via dogfooding? |
||
|
||
namespace OmniSharp.MSBuild | ||
{ | ||
|
@@ -232,14 +234,14 @@ private void WatchProject(ProjectFileInfo project) | |
{ | ||
// TODO: This needs some improvement. Currently, it tracks both deletions and changes | ||
// as "updates". We should properly remove projects that are deleted. | ||
_fileSystemWatcher.Watch(project.FilePath, file => | ||
_fileSystemWatcher.Watch(project.FilePath, (file, changeType) => | ||
{ | ||
OnProjectChanged(project.FilePath, allowAutoRestore: true); | ||
}); | ||
|
||
if (!string.IsNullOrEmpty(project.ProjectAssetsFile)) | ||
{ | ||
_fileSystemWatcher.Watch(project.ProjectAssetsFile, file => | ||
_fileSystemWatcher.Watch(project.ProjectAssetsFile, (file, changeType) => | ||
{ | ||
OnProjectChanged(project.FilePath, allowAutoRestore: false); | ||
}); | ||
|
@@ -378,6 +380,8 @@ private void UpdateSourceFiles(Project project, IList<string> sourceFiles) | |
// Add source files to the project. | ||
foreach (var sourceFile in sourceFiles) | ||
{ | ||
WatchDirectoryContainingFile(sourceFile); | ||
|
||
// If a document for this source file already exists in the project, carry on. | ||
if (currentDocuments.Remove(sourceFile)) | ||
{ | ||
|
@@ -400,6 +404,31 @@ private void UpdateSourceFiles(Project project, IList<string> sourceFiles) | |
} | ||
} | ||
|
||
private void WatchDirectoryContainingFile(string sourceFile) => _fileSystemWatcher.WatchDirectory(Path.GetDirectoryName(sourceFile), OnDirectoryFileChanged); | ||
|
||
private void OnDirectoryFileChanged(string path, FileChangeType changeType) | ||
{ | ||
// Hosts may not have passed through a file change type | ||
if (changeType == FileChangeType.Unspecified && !File.Exists(path) || changeType == FileChangeType.Delete) | ||
{ | ||
foreach (var documentId in _workspace.CurrentSolution.GetDocumentIdsWithFilePath(path)) | ||
{ | ||
_workspace.RemoveDocument(documentId); | ||
} | ||
} | ||
|
||
if (changeType == FileChangeType.Unspecified && File.Exists(path) || changeType == FileChangeType.Create) | ||
{ | ||
// Only add .cs files to the workspace | ||
if (string.Equals(Path.GetExtension(path), ".cs", StringComparison.CurrentCultureIgnoreCase)) | ||
{ | ||
// Use the buffer manager to add the new file to the appropriate projects | ||
// Hosts that don't pass the FileChangeType may wind up updating the buffer twice | ||
Task.Run(() => _workspace.BufferManager.UpdateBufferAsync(new UpdateBufferRequest() { FileName = path, FromDisk = true })); | ||
} | ||
} | ||
} | ||
|
||
private void UpdateParseOptions(Project project, LanguageVersion languageVersion, IEnumerable<string> preprocessorSymbolNames, bool generateXmlDocumentation) | ||
{ | ||
var existingParseOptions = (CSharpParseOptions)project.ParseOptions; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,16 +1,15 @@ | ||
using System.Collections.Generic; | ||
using System.Composition; | ||
using System.Threading.Tasks; | ||
using Microsoft.CodeAnalysis; | ||
using OmniSharp.FileWatching; | ||
using OmniSharp.Mef; | ||
using OmniSharp.Models; | ||
using OmniSharp.Models.FilesChanged; | ||
using System.Collections.Generic; | ||
using System.Composition; | ||
using System.Threading.Tasks; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sort usings with |
||
|
||
namespace OmniSharp.Roslyn.CSharp.Services.Files | ||
{ | ||
[OmniSharpHandler(OmniSharpEndpoints.FilesChanged, LanguageNames.CSharp)] | ||
public class OnFilesChangedService : IRequestHandler<IEnumerable<Request>, FilesChangedResponse> | ||
public class OnFilesChangedService : IRequestHandler<IEnumerable<FilesChangedRequest>, FilesChangedResponse> | ||
{ | ||
private readonly IFileSystemWatcher _watcher; | ||
|
||
|
@@ -20,11 +19,11 @@ public OnFilesChangedService(IFileSystemWatcher watcher) | |
_watcher = watcher; | ||
} | ||
|
||
public Task<FilesChangedResponse> Handle(IEnumerable<Request> requests) | ||
public Task<FilesChangedResponse> Handle(IEnumerable<FilesChangedRequest> requests) | ||
{ | ||
foreach (var request in requests) | ||
{ | ||
_watcher.TriggerChange(request.FileName); | ||
_watcher.TriggerChange(request.FileName, request.ChangeType); | ||
} | ||
return Task.FromResult(new FilesChangedResponse()); | ||
} | ||
|
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.
Sort
System
namespaces first please. You've got the option set incorrectly in VS (this is also the preference in Roslyn).