27

Closed

Adding binding redirects removes comments, changes formatting.

description

Adding binding redirects removes any comments in the <assemblyBinding> sections, and also reformats the config file, changing whitespace in places unrelated to the binding redirects.

For example, this...

<runtime>
<assemblyBinding xmlns="urn:schemas-microsoft-com:asm.v1">
  <!-- Required because Cclp.Presentation was built against a previous version -->
  <dependentAssembly>
    <assemblyIdentity name="Telerik.Windows.Data" publicKeyToken="5803cfa389c90ce7" culture="neutral"/>
    <bindingRedirect oldVersion="2010.3.1314.35" newVersion="2011.1.419.35"/>
  </dependentAssembly>
</assemblyBinding>
</runtime>

<startup>
<!-- Test comment -->
<supportedRuntime 
  version="v2.0.50727"/>
</startup>


...is changed to this...

<runtime>
<assemblyBinding xmlns="urn:schemas-microsoft-com:asm.v1">
  <dependentAssembly>
    <assemblyIdentity name="Telerik.Windows.Data" publicKeyToken="5803cfa389c90ce7" culture="neutral" />
    <bindingRedirect oldVersion="0.0.0.0-2011.1.419.35" newVersion="2011.1.419.35" />
  </dependentAssembly>
</assemblyBinding>
</runtime>
<startup>
<!-- Test comment -->
<supportedRuntime version="v2.0.50727" />
</startup>

Note that:

1) The comment in the <assemblyBinding> section was removed (although the one in the <startup> section was not).
2) The newline between the <runtime> section and the <startup> section was removed.
3) A space was added before the self-closing bracket in the <supportedRuntime> element of the <startup> section.

Removing the comment is destroying important content. The whitespace changes are merely annoying, but it is very important to minimize unnecessary changes when using source control.

file attachments

Closed Jan 29, 2013 at 5:25 PM by danliu
based on the comments below, close the bug now.

comments

Haacked wrote Sep 19, 2011 at 10:26 PM

Please take a look at this one.

ripvannwinkler wrote Mar 23, 2012 at 1:28 PM

I also noticed this. It seems any change to the web.config file causes the entire file to be reformatted. Since I use 4 spaces for tabs, it's incredibly noticeable, because the resulting file has 2 spaces for tabs throughout the entire file. Maybe Nuget simply loads the XML, makes the changes, and then writes it back out, but it should respect existing comments and VS whitespace settings when doing so. This is incredibly important, as it makes it difficult to track single changes in web.config in any VCS, since the whole file will appear to have changed.

dmarsh wrote May 11, 2012 at 4:15 PM

+1. The whitespace conversion is also burning us. We use actual tabs vs. spaces and anytime we add/update packages it'll cause our whole .config formatting to change and we have to remember to go back and CTRL+K+D the document.

jez9999 wrote Oct 3, 2012 at 9:26 AM

I like to use tabs instead of spaces to indent my web.config file, and NuGet reformatting all the indentation is incredibly annoying. There should be a minimalist approach to reformatting, combined with an option to choose how indentation is done.

VisualCSharp wrote Nov 2, 2012 at 4:41 PM

I wholeheartedly agree with the calls to fix this behavior. In fact, to me, this is the single most annoying thing about NuGet. I don't have any issues with the technology except this one.

dotnetjunky wrote Nov 2, 2012 at 5:28 PM

Bringing this bug out of back log to triage. I agree that this is an annoying bug.

maxtoroq wrote Nov 16, 2012 at 1:41 AM

I bet you are using XmlDocument, all you need to do is set PreserveWhitespace = true

tilovell wrote Dec 3, 2012 at 5:01 PM

I think this is more than low impact, it causes too much pain with code reviews.

gromer wrote Dec 24, 2012 at 9:53 PM

Yeah, this is one of the more annoying bugs, would love to see this fixed.

MiroslavBajtos wrote Dec 29, 2012 at 12:28 PM

mello702 wrote Jan 4, 2013 at 9:59 PM

This impacts our development team as well by causing endless formatting issues and costs us time managing web.config merges between developer checkouts.

dotnetjunky wrote Jan 17, 2013 at 1:50 AM

FYI, I have accepted the pull request to fix this bug in NuGet 2.3

danliu wrote Jan 29, 2013 at 12:13 AM

tried with latest build from master branch (2.3). and part of this bug still repros, which is:

3) A space was added before the self-closing bracket in the <supportedRuntime> element of the <startup> section.

see attached web.config files and screenshot for difference.


repro steps:
  1. create a new MVC 4 application
  2. install razorgenerator.testing package
  3. copy original web.config sections from this bug
  4. run add-bindingredirect on PMC

danliu wrote Jan 29, 2013 at 12:14 AM

add another web.config

danliu wrote Jan 29, 2013 at 12:14 AM

add difference screenshot.

MiroslavBajtos wrote Jan 29, 2013 at 6:49 AM

The problem is in an element split on multiple lines:
<supportedRuntime
version="v2.0.50727"/>
which is modified (normalized) to
<supportedRuntime version="v2.0.50727"/>
According to this StackOverflow question & answer, the solution would be rather complex - it would require custom implementation of XML reading, writing and modification.

I don't think the problem is severe enough to justify such an investment. If we believe that somebody will feel like fixing this issue someday in the future, then I am suggesting to create a new issue (preserve spaces between attributes).

dotnetjunky wrote Jan 29, 2013 at 9:00 AM

I agree with MiroslavBajtos. This is a very minor change and I doubt that anyone would mind that. Dan, I don't think this is worth fixing. I'll mark the bug as Fixed and assign it back to you.

Lukazoid wrote Feb 6, 2014 at 9:49 AM

The biggest issue for me is when it writes the Web.config or App.config but has made no changes to any binding-redirects. Incredibly frustrating.