Add the addition and deletion of Miscellaneous files to the Buffer Manager #1277
Conversation
akshita31
requested review from
DustinCampbell,
david-driscoll,
rchande and
filipw
| @@ -128,32 +128,35 @@ private bool TryAddTransientDocument(string fileName, string fileContent) | |||
| var projects = FindProjectsByFileName(fileName); | |||
| if (!projects.Any()) | |||
| { | |||
| return false; | |||
| //todo: deal with the deletion of the misc files | |||
| _workspace.TryAddMiscellaneousDocument(fileName, LanguageNames.CSharp); | |||
There was a problem hiding this comment.
Take a look at the OnWorkspaceChanged handler in this class--you'll want to augment it to remove misc files the way we remove transient documents when they are deleted.
| } | ||
|
|
||
| [Fact] | ||
| public async Task Returns_only_syntactic_diagnotics() |
There was a problem hiding this comment.
I would expect us to still have these tests?
| //todo: deal with the deletion of the misc files | ||
| if (fileName.EndsWith(".cs") && _workspace.TryAddMiscellaneousDocument(fileName, LanguageNames.CSharp) != null) | ||
| { | ||
| _fileSystemWatcher.Watch(fileName, OnMiscFileChanged); |
There was a problem hiding this comment.
Can you store the OnMiscFileChanged delegate in a field so we don't have to create one instance for every file we watch?
| @@ -128,37 +131,54 @@ private bool TryAddTransientDocument(string fileName, string fileContent) | |||
| var projects = FindProjectsByFileName(fileName); | |||
| if (!projects.Any()) | |||
| { | |||
| //todo: deal with the deletion of the misc files | |||
| } | ||
|
|
||
| return true; | ||
| } | ||
|
|
||
| private void OnMiscFileChanged(string filePath, FileChangeType changeType) | ||
| { | ||
| if(changeType == FileChangeType.Unspecified && !File.Exists(filePath) || changeType == FileChangeType.Delete) |
|
@DustinCampbell Does this seem fine ? |
akshita31
changed the title
| @@ -128,37 +133,53 @@ private bool TryAddTransientDocument(string fileName, string fileContent) | |||
| var projects = FindProjectsByFileName(fileName); | |||
| if (!projects.Any()) | |||
| { | |||
| if (fileName.EndsWith(".cs") && _workspace.TryAddMiscellaneousDocument(fileName, LanguageNames.CSharp) != null) | |||
There was a problem hiding this comment.
I was going to say something about adding .csx here but they have different project rules anyway.
There was a problem hiding this comment.
I think we should be doing a case-insensitive EndsWith here to ensure that we also catch .CS.
| @@ -128,37 +133,53 @@ private bool TryAddTransientDocument(string fileName, string fileContent) | |||
| var projects = FindProjectsByFileName(fileName); | |||
| if (!projects.Any()) | |||
| { | |||
| if (fileName.EndsWith(".cs") && _workspace.TryAddMiscellaneousDocument(fileName, LanguageNames.CSharp) != null) | |||
There was a problem hiding this comment.
I think we should be doing a case-insensitive EndsWith here to ensure that we also catch .CS.
Addresses: #1252 (comment)
When the buffer manager adds a transient document, if it is unable to associate the document with any project then the buffer manager will add it as a misc file document.
Doing this will avoid the dependency between the order of execution by the file system watcher listeners, that was introduced when we added the misc files systems.
Also added the filesystem watcher to teh buffer manager so it keeps watching the misc files it added hence can handle the deletion of the same
Known bug: There will be no more code to handle the deletion of these files, hence the diagnostics wont disappear if a misc file is deleted, and in vscode if the user clicks on the diagnostics then we will get the file not found error . Edit: Resolved with using the FileSystemWatcher in the Buffer manager