Removing a key using CLI now actually saves changes to nuget.config

[donnie-msft]

Bug

Fixes: NuGet/Home#8223

Regression? Last working version: No (not in this major version at least)

Description

It seems that removing a key has never actually updated the nuget.config file.

PR Checklist

• PR has a meaningful title

• PR has a linked issue.

• Described changes

• Tests

  • Automated tests added - I added a test, which failed before making this code change, and now it passes.
  • OR
  • Test exception
  • OR
  • N/A

• Documentation

  • Documentation PR or issue filled
  • OR
  • N/A - we already have docs that indicate this behavior, it's just that it didn't work as indicated.

[erdembayar]

            var config = $@"
<configuration>
    <config>
        <add key=""{keyName}"" value=""Highest"" />
    </config>
</configuration>";

[donnie-msft]

I actually agree that this is easier to read, but in recent PR reviews, I've learned it's worse performance. Since this is a test, perf isn't really critical here, and it's probably negligible anyway, but I don't see enough value to rerun CI since it's already green.

[zivkan]

Unless the compiler has improvements I don't know about, interpolated strings (strings starting with $) are syntactic sugar for string.Format, which has much slower runtime performance than string.Concat (which operator +(string a, string b) is syntactic sugar for).

This is a test, so it's not a perf-critical hot-path. But, unless you find concatenated strings much less readable than interpolated strings, I think it's fine as-is.

[erdembayar]

Minor suggestion.

[zivkan]

Unless the compiler has improvements I don't know about, interpolated strings (strings starting with $) are syntactic sugar for string.Format, which has much slower runtime performance than string.Concat (which operator +(string a, string b) is syntactic sugar for).

This is a test, so it's not a perf-critical hot-path. But, unless you find concatenated strings much less readable than interpolated strings, I think it's fine as-is.

[dominoFire]

LGTM. I'm curious how it works with nested nuget.config files.