-
Notifications
You must be signed in to change notification settings - Fork 420
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
Fixes for project file removal support #1694
base: master
Are you sure you want to change the base?
Conversation
savpek
commented
Feb 2, 2020
•
edited
Loading
edited
- Implemented routines to handle removed project files on project system.
- Fixed issues where removal of files (documents/projecfiles) caused state not to update to clients (for example vscode) about removed diagnostics.
Removal of directories is messed up because:
|
@SirIntruder i noticed 😄 microsoft/vscode#90746 I am planning to prototype make fix in omnisharp-vscode side so it will send deleted files 'properly'. It will be hacky and doesn't support direct filesystem changes (made outside of vscode context) but i feel its correct place to fix it since problem is on vscode side instead of making complex workarounds at backend side which is working as intended at the end. In depth theres event that occurs just before move and remove folder events which can be probably be used to write workaround for those cases. However that exist only on newest vscode apis and current omnisharp-vscode has bit obsolete one so there is some refactoring to do. If that ends up as dead end next option is to handle removal of directories so that option at omnisharp-roslyn side where directory removal is handled. But it's bit problematic since there really seem not to be centralized place that understands file structure current state so it requires changes all around the code base probably where all parts handle them properly 🤔 Or some kind of cache between that keeps track of structure but it has risks to go to invalid state that introduce very strange bugs. But yeah i think this problem is one of main reasons why omnisharp requires restarts so often which is quite anonying so even less perfect solution will be step forward. |
But when worked around at vscode side then problems like breaking up during switches between branches persist, so it might be better to do in o-roslyn side at the end, lets see 🤔 |
Ready for review @david-driscoll @filipw @JoeRobich Build pipelines had bad day yesterday 🤔 Will figure out folder removals at another PR, its tricky case because of limitations of VScode side. Another follow up is to support solution file updates properly project added, removed etc (which isn't currently supported). |
@savpek I like the sound of this PR and will make time to review it this week.
Can you walk all the folders and watch each path inidividually. |
Any idea is it possible performance wise? If it doesn't anhilate vscode plugin perf then it could simple and elegant solution for problem 🤔 |
}); | ||
if (projectFileInfo.RuleSet?.FilePath != null) | ||
{ | ||
_fileSystemWatcher.Unwatch(projectFileInfo.RuleSet.FilePath); |
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.
Will this be problematic if multiple projects use the same ruleset file? project.AnalyzerConfigDocuments
are another group of files that could be unwatched, but they are likely shared with other projects so it may require reference counting.
Can I do anything to help getting this PR merged? |