-
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
[WIP] Updating MSBuild for the future #667
[WIP] Updating MSBuild for the future #667
Conversation
5c4e1f2
to
8d4220e
Compare
@@ -330,7 +350,13 @@ private void UpdateSourceFiles(Project project, IList<string> sourceFiles) | |||
continue; | |||
} | |||
|
|||
// If not, add a new document. | |||
// If the source file doesn't exist on disk, don't try to add it. | |||
if (!File.Exists(sourceFile)) |
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.
I wonder if this is needed? Many editors atom/vscode don't require you to save a file to start writing in the file.
It would be great if we could offer a nice editing experience even if the file doesn't yet exist. Though that then has it's own set of problems... that maybe we shouldn't dive into just yet.
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.
This is avoiding a crash below. Unless atom/vscode show you a view of your project, I'm not sure it matters whether we do something fancy in the workspace. This is the scenario where VS would show you a little warning icon in the solution explorer.
4721638
to
44b1c28
Compare
…be copied to output
44b1c28
to
4c6ba03
Compare
Closing this PR. We'll do the work over in the new-msbuild branch. |
DO NOT MERGE!
These are my current changes to support MSBuild 15.0 and run OmniSharp on an embedded Mono. As mentioned in #666, there's a fair amount of work to sort out how OmniSharp will acquire its Mono and MSBuild payloads, so this can't be simply merged.