Improved Solution File Handling #96
Conversation
andreiborodin
added
enhancement
andreiborodin
requested review from
CoPrez,
chrisfromwork,
mhochk and
ryantrem
...Unity/Packages/com.microsoft.msbuildforunity/Editor/ProjectGenerator/Scripts/MSBuildTools.cs
Show resolved
Hide resolved
...Unity/Packages/com.microsoft.msbuildforunity/Editor/ProjectGenerator/Scripts/MSBuildTools.cs
Show resolved
Hide resolved
| @@ -0,0 +1,395 @@ | |||
| // Copyright (c) Microsoft Corporation. All rights reserved. | |||
| // Licensed under the MIT License. See LICENSE in the project root for license information. | |||
|
|
|||
There was a problem hiding this comment.
nit: There's a newer copyright header #Resolved
There was a problem hiding this comment.
| guid = config.AssemblyCSharpFirstPassEditorGuid; | ||
| break; | ||
| default: | ||
| throw new InvalidOperationException($"Predefined assembly '{assemblyDefinitionInfo.Name}' was not recognized, this generally means it should be added to the switch statement in CSProjectInfo:GetProjectType. Treating is as a PredefinedAssembly instead of PredefinedEditorAssembly."); |
There was a problem hiding this comment.
is [](start = 248, length = 2)
I think it is supposed to be "Treating it as a PredefinedAssembly" not "is" #Pending
There was a problem hiding this comment.
You are right, however, the message is actually stale now, since I just throw. I will update the message, thanks for flagging it. #Resolved
nit: consider moving these variables into a wrapper struct/class to decrease the number of arguments in this function call #WontFix Refers to: Source/MSBuildTools.Unity/Packages/com.microsoft.msbuildforunity/Editor/ProjectGenerator/Scripts/Exporters/TemplatedProjectExporter.cs:56 in 283a6d5. [](commit_id = 283a6d5, deletion_comment = False) |
| } | ||
| } | ||
|
|
||
| private SortedDictionary<string, SortedSet<string>> GetSolutuonConfigurationPlatformMap(UnityProjectInfo unityProjectInfo, SolutionFileInfo solutionFileInfo) |
There was a problem hiding this comment.
GetSolutuonConfigurationPlatformMap [](start = 60, length = 35)
nit: "GetSolutuonConfigurationPlatformMap" should be "GetSolutionConfigurationPlatformMap" #Resolved
There was a problem hiding this comment.
...ages/com.microsoft.msbuildforunity/Editor/ProjectGenerator/Scripts/AssemblyDefinitionInfo.cs
Show resolved
Hide resolved
...Unity/Packages/com.microsoft.msbuildforunity/Editor/ProjectGenerator/Scripts/MSBuildTools.cs
Show resolved
Hide resolved
...Unity/Packages/com.microsoft.msbuildforunity/Editor/ProjectGenerator/Scripts/MSBuildTools.cs
Show resolved
Hide resolved
...Unity/Packages/com.microsoft.msbuildforunity/Editor/ProjectGenerator/Scripts/MSBuildTools.cs
Show resolved
Hide resolved
| public Dictionary<string, SolutionFileSection<SolutionProjecSectionType>> Sections { get; set; } | ||
| } | ||
|
|
||
| internal static class TextSolutionFileParser |
There was a problem hiding this comment.
TextSolutionFileParser [](start = 26, length = 22)
I haven't looked much in to it, but just making sure you're aware of the Microsoft functionality around solution/project parsing.
https://docs.microsoft.com/en-us/previous-versions/visualstudio/visual-studio-2013/dn763266(v=vs.121
It looks like it may have been moved to be internal-only functionality still accessible through reflection, so possibly not the best direction, but just making sure it is known about. #WontFix
There was a problem hiding this comment.
Yes, here is the problem, I need to reference a NuGet package for that. This presents a form of Catch 22 as this project is responsible for bringing the package in. There is a potential workaround, but we will figure that part out later.
In reply to: 364913591 [](ancestors = 364913591)
This change makes a lot of improvements to how MSB4U solution file is generated, and more importantly what information it tries to bring forward when overwriting this file. This means, you can now modify and add items to the solution file, and have MSB4U preserve those.
This change addresses:
#86