Add support for Miscellaneous Files

[akshita31]

Fixes: #207

[filipw]

very promising 😃

[DustinCampbell]

Naming: remove the "I" and go with "ExportProjectSystemAttribute".

[SirIntruder]

I built this, and omnisharp.exe now takes 5.5GB of ram to process my work project, (~200 files marked as misc) 😲

before it looked like this:

[SirIntruder]

I did a quick prototype to replace ConcurrentDictionary<string, ProjectInfo> with one ProjectInfo and a lock, and the problem is gone. Any reason why one-project-per-file is needed? Just having one project file seems both simpler, more performant and a better service to the user (cross references between misc files work)

edit: lol, I see what's the issue here... intent was to create one misc project per language, but GetOrAdd() call used CreateMiscFile() as a value instead of a Func<>, creating new project info over and over again 😄 I submitted PR

[DustinCampbell]

@SirIntruder: It shouldn't have been done that way. I'd asked about this awhile back and it was concluded to be OK because VS does this for miscellaneous files. However, VS only does this for open files -- not every closed file not contained in a project.

[DustinCampbell]

@rchande: FYI I would have appreciated another opportunity to take a look at this before it was merged. @akshita31 had asked me, @filipw and @david-driscoll if it was OK last Friday. Then, it got merged after the weekend on Monday?

I'm concerned that this will cause a significant regression: 81321cd#r210019117

[rchande]

@DustinCampbell My apologies. Next time, we'll be more careful about waiting for everyone to get a chance to review. @akshita31 and I are happy to address any post-facto feedback you have on this one. It's hard to tell exactly from the link you added but I think the concern is that the MiscFiles project system is eagerly scanning for every .cs file under the cone? I suppose the way to make this lazier would be to allow editors to drive the behavior. I guess this could work at least a couple of ways:

• Requests (copmletion, etc) sent for files not in the Workspace result in O# automatically adding that file as a misc file
• We add a new endpoint "FileOpened" or similar that editors can send requests to. If a file is opened that's not in the workspace already, it gets added as a misc file.

Thoughts?

[DustinCampbell]

It's hard to tell exactly from the link you added but I think the concern is that the MiscFiles project system is eagerly scanning for every .cs file under the cone? I suppose the way to make this lazier would be to allow editors to drive the behavior.

My concern is that the MiscFilesWorkspace eagerly adds files based on file changed events. Consider the following scenario:

• The user opens an existing C# project in OmniSharp. The C# project has file globs to determine what files are included (for example, an ASP.NET Core project).
• The user creates a new C# file in VS Code in their project folder, expecting it to be included in the project.
• They are surprised to find that their editor experience in that file is terrible because it didn't get included in their project -- it got included in the MiscFilesWorkspace.
• The user has to restart OmniSharp to fix the problem.

To be clear, the MiscFiles workspace should be a fallback. As implemented, I believe it will "steal" files that belong to projects.

Prior to this change, newly-created files wouldn't be included in any project until the user edited them. When OmniSharp's BufferManager got notified of the edit, it would locate a project in which to include the file if it wasn't in a project already. Since the MiscFilesWorkspace will have already included it in a project, I suspect that code won't work.

Does that make sense? I think there's a significant experience regression that'll be caused by this.

[akshita31]

@DustinCampbell Apologies for merging this without your review.
Your concern completely makes sense. I am investigating further into this.

[akshita31]

@DustinCampbell @rchande
We tried to address this problem by adding a wait for the update of the project systems in the MiscFilesSystem.
But I now understand that if the update for the MiscFiles is invoked first, then the system will add the file as a MiscFile and the BufferManager will not be able to do the right thing with it.

An alternative that comes to my mind is that instead of creating a whole project system for the MiscFiles we could add the document as a "misc document" when the buffer manager is unable to add it as a transient document:

omnisharp-roslyn/src/OmniSharp.Roslyn/BufferManager.cs

Line 131 in 7e684d4

return false;

. However if we do it that way, how do we handle the deletion of these files?

To the best of my understanding, currently all the project systems handle the deletions for the files that they "care" about. But if we add the handling of addition of files to the buffer manager, who will handle the deletion? Is it possible for the BufferManager to handle it?

[SirIntruder]

Omnisharp now crashes every time you open deleted file in git side bar ._.

Unhandled Exception: System.NotSupportedException: The given path's format is not supported.
at System.Security.Permissions.FileIOPermission.EmulateFileIOPermissionChecks(String fullPath)
at System.Security.Permissions.FileIOPermission.QuickDemand(FileIOPermissionAccess access, String fullPath, Boolean checkForDuplicates, Boolean needFullPath)
at System.IO.FileInfo.Init(String fileName, Boolean checkHost)
at OmniSharp.MiscellaneousFile.MiscellaneousFilesProjectSystem.d__19.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
at System.Threading.ExecutionContext.RunInternal(ExecutionContext executionContext, ContextCallback callback, Object state, Boolean preserveSyncCtx)
at System.Threading.ExecutionContext.Run(ExecutionContext executionContext, ContextCallback callback, Object state, Boolean preserveSyncCtx)
at System.Threading.QueueUserWorkItemCallback.System.Threading.IThreadPoolWorkItem.ExecuteWorkItem()
at System.Threading.ThreadPoolWorkQueue.Dispatch()

[akshita31]

@SirIntruder The crash was because the extension was sending incorrect file paths for deleted git files. I have created a PR in omnisharp-vscode to address that: dotnet/vscode-csharp#2465.

[akshita31]

@DustinCampbell Can you take a look ?

[DustinCampbell]

@akshita31 : What should I take a look at? This is a pretty big PR.

[DustinCampbell]

Also, this is a merged PR. Not sure what I could be taking a look at. 😄

[akshita31]

@DustinCampbell This comment: #1252 (comment)

[DustinCampbell]

What you suggested in that comment makes sense to me. I think a project system is probably overkill and introduces some ordering concerns.

[gulbanana]

Is there a setting to disable miscellaneous files support? I have some solutions where I want OmniSharp only to load certain projects (because the others are Windows-specific).

[akshita31]

@gulbanana Miscellaneous files support doesn't load any projects. It is only meant to provide basic intellisense support for files without a project or files that have not been loaded as part of the currently loaded project in omnisharp. Also, the misc file is added to the workspace only when the user edits the file, hence if there are miscellaneous files in your workspace that you are not editing, misc files support won't do anything about it.

[bklebe]

Hi all,

I'm working on a bug (OmniSharp/omnisharp-emacs#464) that shares many similarities with dotnet/vscode-csharp#785. I've hunted through the elisp files and I think this bug is upstream of omnisharp-emacs. I found this PR while looking through diffs in the 1.32.x series because it seemed relevant. Is it possible that new buffers are getting initially loaded as misc files, causing namespace duplication errors when the current solution finishes loading? These bugs do not seem to be a problem with files that are actually outside of a project structure.

Apologies if I've got something horribly wrong, I must admit my main area of expertise is OCaml, not C#.

[rchande]

@beeuhtricks We recently took a fix (#1357) for this issue. Can try out one of our latest builds and see if that fix works for you? Thanks!

[bklebe]

@rchande I tried it with 1.32.9-beta.20/omnisharp-osx.tar.gz and it seems to have fixed the problem. Thank you so much!

[bklebe]

Hi there, We’re getting reports that this issue persists, albeit occasionally, with v1.32.10. I would be happy to work with you on a solution if necessary.

…

________________________________ From: Ravi Chande <notifications@github.com> Sent: Tuesday, December 18, 2018 11:01 AM To: OmniSharp/omnisharp-roslyn Cc: Beatrix Klebe; Mention Subject: Re: [OmniSharp/omnisharp-roslyn] Add support for Miscellaneous Files (#1252) @beeuhtricks<https://github.com/beeuhtricks> We recently took a fix (#1357<#1357>) for this issue. Can try out one of our latest builds and see if that fix works for you? Thanks! — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub<#1252 (comment)>, or mute the thread<https://github.com/notifications/unsubscribe-auth/AIpKPc99U3E7A_FN8YN2mn-LTZNP1f47ks5u6RE9gaJpZM4VnEuA>.