Round trip formatting / comments

[AndyGerlicher]

This issue is in the open design phase and is a part of the project.json -> csproj conversion effort.

We should preserve user authored content/whitespace in project files when saving.
E.g.

<PropertyGroup>
  <TargetFrameworkVersion>v4.5.2</TargetFrameworkVersion> <!-- 4.5.2 for back-compat -->
</PropertyGroup>

Today will be re-saved as:

<PropertyGroup>
  <TargetFrameworkVersion>v4.5.2</TargetFrameworkVersion> 
  <!-- 4.5.2 for back-compat -->
</PropertyGroup>

We should investigate to see if there's an easy win here, but we should not be creating our own XML writer for this release.

[davkean]

It not only changes the location of comments, it also changes indentation, and removes non-significant whitespace.

I did some investigation, and looks like this is going to be an easy fix to preserve whitespace/comments, etc. CPS calls through ProjectRootElement Open(string path, ProjectCollection projectCollection) this path causes XmlDocument.PreserveWhitespace to default to false here. If CPS has an overload where it could opt-into preserving whitespace (you'll probably want that for compat, rather than turning it on by default), then we can avoid changing the underlying project file.

[davkean]

It doesn't have to be an overload, it could just a mutable property on the ProjectRootElement, or however, you want to design it.

[dsplaisted]

@davkean Does CPS set the MSBUILDCACHECHECKFILECONTENT environment variable? The code you pointed to is only active if that variable is set.

[dsplaisted]

I have a work in progress fix for this at https://github.com/dsplaisted/msbuild/tree/AllowPreserveWhitespace

It turns out that single quotes are being converted to double quotes when a project is saved. IE this:

<ProjectReference Include='..\CLREXE\CLREXE.vcxproj'>

Will be converted to this:

<ProjectReference Include="..\CLREXE\CLREXE.vcxproj">

I haven't figured out where the quote conversion is happening yet.

[davkean]

Not sure the relevance of MSBUILDCACHECHECKFILECONTENT, from what I see - it's just a test hook.

Not too worried about the single quote -> double quote conversion, I wouldn't block the check-in on that (it's probably in the XML writer). Can we get this in soon?