This repository has been archived by the owner on Oct 4, 2021. It is now read-only.
File watcher integration #5040
Merged
File watcher integration #5040
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Previously the native file watcher was statically linked into monostub. This meant that the unit tests were not using the native file watcher but the one provided by Mono. Switching to a dylib allows the unit tests to use the native file watcher. If the dylib is missing then the Mono file watcher will be used.
Added test that checks the native file watcher is available in unit tests.
Added a simple way to register a solution so all files in its directories are monitored. Only file change events are passed to the FileService which are then raised as events by the FileService. Added some tests to verify that modify a project inside a solution or a file inside the project generates FileService FileChanged events. The tests that use Project.SaveAsync pass without the new file watcher since the FileService is called directly to notify file changes when SolutionItem.DoSave is called and MSBuildProject.SaveAsync is called, which uses TextFile.Write which generates the event. The direct calls to notify the FileService have not yet been removed.
Registering a solution with the file watcher will generate file remove events from the FileService. Note that currently file remove events are generated on saving a file which has not yet been addressed. The new tests also seem to break the SaveFileInProjectExternally test - no change event is fired when saving a file using TextFileUtility.WriteText.
The TextFileUtility creates a temp file in the same directory as the file being saved but changes the filename so it starts with '.#' The native file watcher on Mac does not always generate a change event when a file is saved using the TextFileUtility so we treat a rename event of the temp file to the file being saved as a change event that will be raised by FileService.FileChanged. This fixes the SaveFileInProjectExternally test that was failing.
On opening a solution it has a file watcher created. When the solution is disposed the file watcher is removed.
Removed the code that checked all the files belonging to open solution when the IDE lost focus, or a build was started, then checked for modified or removed files on regaining focus or the build completes. This is now handled by the file watcher for the solution. Note that in the case of the IDE losing focus the file events are frozen until the IDE gets focus again.
Check the last write time of the file before reloading it to handle change events being generated by the file watcher. Also no need to generate a file change event directly in the Document class since one is generated by the file watcher.
When different FileService event types (e.g. copy followed by a change event) occurred when the FileService was frozen on thawing the attempt to merge similar events together would fail since the cast to EventData<TArgs> would not work for different event types. System.InvalidCastException: Specified cast is not valid. at MonoDevelop.Core.EventQueue+EventData`1[TArgs].ShouldMerge (MonoDevelop.Core.EventQueue+EventData other) in main/src/core/MonoDevelop.Core/MonoDevelop.Core/FileService.cs:865 at MonoDevelop.Core.EventQueue.Thaw ()
Handle projects and files that exist outside the solution directory when watching files. File watchers are now created for all directories that are needed instead of just for the solution's directory. Currently the directory information is cached in the WorkspaceItem and not updated when a new project or file is added or removed.
The test was directly removing the solution from the file watcher service. Now the solution is disposed which will remove the solution from the file watcher.
Initial simplistic handling of multiple solutions being opened which share a common directory.
Ensure that the directories watched are normalized so a directory is not monitored more than once which would result in duplicate file events being generated.
This fixes the file watcher being disabled for a solution when another solution was closed and exists in a directory that is a parent of the other solution.
Allow directories to be watched without having to have them belonging to a solution. This allows documents to be monitored for changes when they do not belong to a solution.
Previously the workspace item was registering itself with the file watcher. This was causing tests to hang since some solutions are not disposed.
Adhoc solutions are not registered with the file watcher and when they were disposed were triggering an update of the file watchers. Now a check is made to see if the solution is being watched before updating the file watchers.
Refresh the file watchers after the file in the text editor is saved as another file somewhere else.
Refresh the directories monitored when a project is added to a solution or removed from a solution.
Refresh the file watchers when the workspace has a solution added to it or removed from it.
When a file is added to a project or removed from a project the file watchers are now refreshed. Currently the logic is very simple - any file added or removed will cause a refresh. This could be improved.
Some applications, such as TextEdit.app, will create a backup file and then rename that to the original file. This results in no file change event being generated by the file watcher. To handle this a rename is treated as a file change for the destination file.
The FileService was merging the file events to the previous event not the next one. So if there was one change for file 1 followed by a change for file 2 the FileService would lose the change for file 1.
File events are generated now for all open documents and solutions so the GetFileStatusTracker is not needed.
The file watcher will now generate the required Changed and Deleted events. Freezing and Thawing the FileService to get the same behaviour as before when the FileService notifications were applied after the stash was applied. Behaviour is a bit broken here compared with before. The native file watcher seems to want to generate - FileChanged, FileCreated and FileDeleted. This results in any open documents that were changed by the stash to be closed. Previously these would be reloaded. The native file watcher gets a changed, created and deleted event flag in the same message and then fires three events - changed, created and deleted event even though the file exists.
The native file watcher sometimes generates Changed, Created and Deleted events in that order from a single native file event. This can occur even when the file is not deleted. Sometimes you can trigger this behaviour using File.WriteAllText when the file does not exist. Now a check is made to ensure the file does not exist before the file watcher deleted event handler raises the FileRemoved event with the FileService.
The Git StatusView indicates it has a file but the parent directory of the filename is null. This was not handled by the file watcher and resulted in the Git status window not being displayed. In the IDE log an IndexOutOfRangeException was thrown: Error while executing command: Review Solution and Commit System.IndexOutOfRangeException: Index was outside the bounds of the array. at MonoDevelop.Core.FilePath.IsChildPathOf (MonoDevelop.Core.FilePath basePath) in MonoDevelop.Core/FilePath.cs:180
Deleting a file externally on the Mac is treated as a move to the ~/.Trashes directory. This rename event was being ignored so the file was not being closed on switching back to the IDE. Now a delete event is fired by the FileService for the original file when it is renamed.
Remove calls to FileService.NotifyFileChanged since the file watcher will generate these events. RootWorkspace's FileStatusTracker still has calls that directly use the FileService however this class is obsolete and not used anywhere. The WelcomePageSection makes a direct call to FileService's NotifyFileRemoved. This would not be generated by the file watcher since the project may have been deleted before the IDE was opened. This call is a way to have the project removed from the recently used list.
Remove calls to FileService.NotifyFileChanged. The file watcher will generate these events now.
Removed calls to FileService.NotifyFileChanged when generating the web reference files. The file change events are raised by the file watcher.
Remove calls to FileService.NotifyFileChanged when refactoring changes code in files which are not open. The file watcher will generate file change events for these changes.
Removed use of FileService.NotifyFileChange from the CodeGenerationService. Only code that seems to call this is the GtkCore addin in the ActionGroupDisplayBinding. That code does not seem to work - it fails to create a valid syntax tree in ActionGroupDisplayBinding.CreateClass, the C# code analysis SyntaxFactory.GetConstructorInitializerThisOrBaseKeywordKind throws an ArgumentOutOfRange exception. Also the names used are empty strings.
When there are a lot of FileService.FileRemoved events being generated the UI would hang whilst the favourited projects information was being saved in the properties file. Now a check is made to see if the favourites have changed and if not a save is not done. This fixes the UI hang if you remove a node_modules folder inside a non .NET Core project that contains thousands of files.
Removed calls to the FileService for file change and remove events. Removed calls that directly reload documents in the IDE.
Remove calls to FileService.NotifyFileChanged when the makefile is updated. The file watcher will fire the file change events for makefiles in the project or solution directory.
Remove calls to FileService.NotifyFileChanged. The file change events will be raised by the file watcher.
Removed calls to FileService.NotifyFileChanged. The file change events are not generated by the file watcher.
Also fix typo in file watcher comment.
mrward
requested review from
DavidKarlas,
mkrueger,
sevoku and
slluis
as code owners
slluis
reviewed
Jun 12, 2018
| @@ -173,7 +173,9 @@ protected override void Run () | |||
| } | |||
| finally { | |||
| monitor.Dispose (); | |||
| statusTracker.Dispose (); | |||
| Runtime.RunInMainThread (delegate { | |||
| FileService.ThawEvents (); | |||
There was a problem hiding this comment.
We can just do the RunInMainThread call inside ThawEvents() if necessary.
slluis
reviewed
Jun 12, 2018
| if (IdeApp.IsInitialized) { | ||
| MonoDevelop.Ide.Gui.Document doc = IdeApp.Workbench.GetDocument (fp); | ||
| if (doc != null) | ||
| doc.Reload (); |
There was a problem hiding this comment.
I think we still need this. If the user executes an operation that results in a file being updated, that file should be automatically reloaded in the editor if it is already open, it shouldn't notify the user that the file has changed.
slluis
reviewed
Jun 12, 2018
| // Reload reverted files | ||
| Document doc = IdeApp.Workbench.GetDocument (item.Path); | ||
| if (doc != null && System.IO.File.Exists (item.Path)) | ||
| doc.Reload (); |
There was a problem hiding this comment.
Same as my previous comment about Reload. The goal here is to avoid prompting the user about file change.
|
Updated the pull request.
|
The ThawEvents method will now handle being called from a non-UI thread.
The FileService.ThawEvents method will now handle being called from a background thread.
Add back original code that reloaded reverted files that were open in the text editor. Without this change if the file is dirty then the file will not be reloaded and instead the text editor will show a message about the file being changed outside the IDE. Since the user explicitly asked to revert the file then the file should be reloaded automatically even if there are unsaved changes.
mrward
force-pushed
the
file-watcher-integration
branch
from
c8c7d31
to
a3d9526
Compare
slluis
approved these changes
Jun 13, 2018
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Use native file watcher to monitor solutions and individual files opened in the IDE.
Fixes VSTS #593841 - IDE focus-in/out does a lot of IO
Fixes VSTS #619697 - UI freeze after opening solution
Fixes VSTS #619707 - Git pull results in inconsistent project status