Skip to content
This repository has been archived by the owner on Oct 4, 2021. It is now read-only.

[IDE] Use a FileSystemWatcher to take changes made while unfocused #4400

Closed
wants to merge 1 commit into from

Conversation

iainx
Copy link
Contributor

@iainx iainx commented Mar 30, 2018

Track file changes made while unfocused using a FileSystemWatcher instead of
stating every file for the modification time when the application loses focus
and then again when it regains focus

Fixes GH #4383

Track file changes made while unfocused using a FileSystemWatcher instead of
stating every file for the modification time when the application loses focus
and then again when it regains focus

Fixes GH #4383
@iainx iainx requested review from Therzok and mkrueger March 30, 2018 14:23
@iainx iainx requested a review from slluis as a code owner March 30, 2018 14:23
@Therzok
Copy link
Contributor

Therzok commented Mar 30, 2018

Is the mono FSW using the corefx version right now? We need to rely on a FSW which uses the corefx version.

Which means:

  • Either Mono > $version
  • Use MD.Core one in MD.Ide (but we don't have IVT, not sure if we want that)
  • Copy the MD.Core one into MD.Ide and use it until stable mono has it.

watchers = new List<FileSystemWatcher> ();

// Take each workspace item and watch its base directory for any file changes
foreach (WorkspaceItem item in IdeApp.Workspace.Items) {
Copy link
Contributor

Choose a reason for hiding this comment

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

We shouldn't create and get rid of a file watcher every time we focus in/out.

We should probably iterate the solution and solution items and find the base common dir. If all paths are children of the common dir, just monitor the common dir.

Otherwise make multiple file watchers for all the paths: i.e. solution A has a project in $(slndir)/../project, we monitor $slndir and $(slndir)/../project.

In focus in/out should just toggle enable raising events.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ohh, that's for every solution. Okay, that's cool, but we should just keep the FSWs alive and toggle enableraisingevents.

We can use WorkspaceitemOpened to switch the paths we monitor.

Copy link
Contributor

Choose a reason for hiding this comment

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

From slack:

iain holmes [7:24 PM]
I can move the watcher to the WorkspaceItem

Which is an even better idea!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In fact, if we rewrite WorkspaceItem.FileStatusTracker to use FileSystemWatcher like it sort of was planned (all the necessary code is commented out) then we could do away with the FocusIn/FocusOut tracking altogether, no?

@Therzok
Copy link
Contributor

Therzok commented Apr 3, 2018

So, it seems like the mono corefx FSW is only available in master atm.

I think the simplest solution we have for now is to split MonoDevelop.FSW into a different assembly and give IVT to Core and IDE so we don't expose this as API. (cc @mrward )

@slluis
Copy link
Member

slluis commented Apr 4, 2018

This is not the right approach. We need to use MonoDevelop.FSW inside the project model or FileSystemService. The check for changes on focus-in/out was a workaround for the lack of efficient FSW, but it is not the right solution now that we have good FSW.

@iainx
Copy link
Contributor Author

iainx commented Apr 4, 2018

@slluis yes, that's what I was thinking. If I was to rewrite WorkspaceItem.FileStatusTracker to use the good FSW, that would remove the need for the focus-in/out workaround.

Would that work instead of putting it into the project model?

@mkrueger
Copy link
Contributor

mkrueger commented Apr 5, 2018

The focus in/focus out approach was for a non working FSW - when using for example VIM the FSW didn't throw a file change notification - was very strange. Some editors/file changes did some did not.

VS.NET does the file update in their project system AFAIK.


// Take each workspace item and watch its base directory for any file changes
foreach (WorkspaceItem item in IdeApp.Workspace.Items) {
var watcher = new FileSystemWatcher (item.BaseDirectory);
Copy link
Contributor

@mkrueger mkrueger Apr 5, 2018

Choose a reason for hiding this comment

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

Projects can contain files that are outside of the base directory of the project.

@mkrueger
Copy link
Contributor

mkrueger commented Apr 5, 2018

That patch doesn't work for files that are not in a project. The old approach was just a work around but was correct in that use case.

@Therzok
Copy link
Contributor

Therzok commented Apr 26, 2018

This will need to be rebased so it works with new CI lanes.

@Therzok
Copy link
Contributor

Therzok commented May 31, 2018

@mrward is this fixed in file-watcher-integration PR?

@mrward
Copy link
Member

mrward commented Jun 1, 2018

@Therzok - Yes this is fixed on the file-watcher-integration branch. I guess we can close this.

@Therzok Therzok closed this Jun 16, 2018
@Therzok Therzok deleted the fix-4383 branch June 16, 2018 02:22
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants