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

Listen for file renames #987

Merged
merged 28 commits into from
Oct 27, 2017
Merged
Show file tree
Hide file tree
Changes from 21 commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
70f8486
Add a dedicated FilesChangedRequest type that includes the change tha…
Oct 24, 2017
47804cf
Update IFileSystemWatcher to:
Oct 24, 2017
2459ea0
Implement new FileWatcher members
Oct 24, 2017
1096cd6
Consume better file change notifications from MSBuild project system
Oct 24, 2017
c46fad5
Update namespaces
Oct 24, 2017
2f2ea4c
Add a test
Oct 24, 2017
d5a8477
Delete dead files
Oct 24, 2017
e9e1fd7
Comment
Oct 24, 2017
2882a33
Add using block to fix test
Oct 24, 2017
c9db400
verb -> changeType
Oct 25, 2017
634be55
FileChangeType -> ChangeType
Oct 25, 2017
4db48c7
Remove FileChangeType?
Oct 25, 2017
9847dc4
Sort usings
Oct 25, 2017
2b19763
Merge branch 'master' into fileWatching
Oct 25, 2017
c7884fe
Merge branch 'master' into fileWatching
DustinCampbell Oct 25, 2017
b3cb182
Wait for buffer update in order to be testable
Oct 25, 2017
3ef284e
Sort usings
Oct 25, 2017
14e9626
Allow multiple clients to watch the same directory
Oct 25, 2017
b122830
Add a test for multiple directory watchers
Oct 25, 2017
9802d7e
Merge branch 'fileWatching' of https://github.com/rchande/omnisharp-r…
Oct 25, 2017
9796a50
sort usings
Oct 25, 2017
e8652fa
Use delegate combining syntax
Oct 25, 2017
9b96a8d
Merge branch 'master' into fileWatching
DustinCampbell Oct 26, 2017
f305c86
Merge branch 'master' of https://github.com/OmniSharp/omnisharp-rosly…
Oct 26, 2017
36c145b
Don't attempt to add Documents for directory names that end in .cs
Oct 26, 2017
5d0b91b
Merge branch 'master' into fileWatching
Oct 26, 2017
858bcbb
Merge branch 'fileWatching' of https://github.com/rchande/omnisharp-r…
Oct 26, 2017
259bc76
Merge branch 'master' into fileWatching
Oct 26, 2017
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
Empty file added changeType
Empty file.
14 changes: 11 additions & 3 deletions src/OmniSharp.Abstractions/FileWatching/IFileSystemWatcher.cs
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
Expand Up @@ -3,6 +3,9 @@

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; }
Copy link
Member

Choose a reason for hiding this comment

The 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 Newtonsoft.Json)

}
}
4 changes: 2 additions & 2 deletions src/OmniSharp.DotNet/DotNetProjectSystem.cs
Original file line number Diff line number Diff line change
Expand Up @@ -166,13 +166,13 @@ private void UpdateProject(string projectDirectory)
_projectStates.Update(projectDirectory, contexts, AddProject, RemoveProject);

var projectFilePath = contexts.First().ProjectFile.ProjectFilePath;
_fileSystemWatcher.Watch(projectFilePath, file =>
_fileSystemWatcher.Watch(projectFilePath, (file, changeType) =>
{
_logger.LogInformation($"Watcher: {file} updated.");
Update(allowRestore: true);
});

_fileSystemWatcher.Watch(Path.ChangeExtension(projectFilePath, "lock.json"), file =>
_fileSystemWatcher.Watch(Path.ChangeExtension(projectFilePath, "lock.json"), (file, changeType) =>
{
_logger.LogInformation($"Watcher: {file} updated.");
Update(allowRestore: false);
Expand Down
51 changes: 0 additions & 51 deletions src/OmniSharp.Host/FileWatching/FileSystemWatcherWrapper.cs

This file was deleted.

34 changes: 27 additions & 7 deletions src/OmniSharp.Host/FileWatching/ManualFileSystemWatcher.cs
Original file line number Diff line number Diff line change
@@ -1,24 +1,44 @@
using System;
using System.Collections.Generic;
using System.IO;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sort usings so System namespaces are first.

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>>();
Copy link
Contributor

Choose a reason for hiding this comment

The 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 Delegate.Combine(...) and Delegate.Remove(...)?

Copy link
Author

Choose a reason for hiding this comment

The 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)
{
if (_directoryCallBacks.TryGetValue(path, out var existingCallback))
{
_directoryCallBacks[path] = (Action<string, FileChangeType>)Delegate.Combine(callback, existingCallback);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm OK leaving this as is, but it's also possible to just write:

_directoryCallBacks[path] = callback + existingCallback;

Believe it or not, + handles combination of delegates in C#. 😄

The More You Know

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So much nicer. Fixing.

}
else
{
_directoryCallBacks[path] = callback;
}
}
}
}
}
81 changes: 0 additions & 81 deletions src/OmniSharp.Host/Services/WorkspaceHelper.cs

This file was deleted.

3 changes: 2 additions & 1 deletion src/OmniSharp.MSBuild/MSBuildHelpers.cs
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,8 @@ public static bool CanInitializeVisualStudioBuildEnvironment()

var mode = GetPropertyValue("Mode", buildEnvironment, s_BuildEnvironmentType, BindingFlags.NonPublic | BindingFlags.Instance);

return mode?.ToString() == "VisualStudio";
// return mode?.ToString() == "VisualStudio";
return false;
}
}
}
33 changes: 31 additions & 2 deletions src/OmniSharp.MSBuild/MSBuildProjectSystem.cs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@
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;
Expand Down Expand Up @@ -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);
});
Expand Down Expand Up @@ -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))
{
Expand All @@ -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
_workspace.BufferManager.UpdateBufferAsync(new UpdateBufferRequest() { FileName = path, FromDisk = true }).Wait();
}
}
}

private void UpdateParseOptions(Project project, LanguageVersion languageVersion, IEnumerable<string> preprocessorSymbolNames, bool generateXmlDocumentation)
{
var existingParseOptions = (CSharpParseOptions)project.ParseOptions;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,12 @@
using Microsoft.CodeAnalysis;
using OmniSharp.FileWatching;
using OmniSharp.Mef;
using OmniSharp.Models;
using OmniSharp.Models.FilesChanged;

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;

Expand All @@ -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());
}
Expand Down
Loading