Are we sure that all of the other MSBuild options also support the "comma delimited values with a single key" format that WarningsNotAsErrors does? Because the GetPropertyArguments method is used for all properties..

Unfortunately I can't even find documentation confirming that duplicate /property assigns to the same key will overwrite, I'm just inferring from results.

I've looked through the msbuild switches, compiler options and common build parameters. In most cases it's comma or semicolon delimitation, but in some it's only comma, and some only semicolon.
So you're correct and a complete switch to comma delimitation would most likely break things. Although I'm not certain that these would work currently anyway, as I've not yet seen a cakescript utilising a WithProperty with multiple values.

As a change is not really viable, possibly another method could be added to account for comma delimited properties?

 public static MSBuildSettings WithCommaDelimitedProperty(this MSBuildSettings settings, string name, params string[] values)
        {
            if (settings == null)
            {
                throw new ArgumentNullException("settings");
            }

            IList<string> currentValue;
            currentValue = new List<string>(
                settings.Properties.TryGetValue(name, out currentValue) && currentValue != null
                    ? currentValue.Concat(values)
                    : values);

            settings.Properties[name] = String.Join(",", currentValue);

            return settings;
        }

It would let GetPropertyArguments function the same, while dealing with the comma delimited case.

This will need to be investigated, we could probably step thru the MSBuild source and see what it does. If someone wants to contribute / do the work I'll happily review a PR.

HI @devlead, i have step thru MSBuild sources and it seems that they support either comma's or semicolons (,;) as a separator if a switch has multiple values. See here. So i think it's fine what @nickmacd has suggested that we can switch to comma separation with a small change around values. You have to set it between quotes.

foreach (var propertyKey in properties.Keys)
{
var values = string.Join(",", properties[propertyKey]);
yield return string.Concat("/p:", propertyKey, "=",  '"', ,values, '"');
}

And as you see here, duplicate properties are always will overwrite.

@devlead: confirming what @martinscholz83 and @nickmacd found is still valid.

• Calling ".WithProperty("WarningsNotAsErrors", "1573", "1591")" results in cake executing MSBuild with "/p:WarningsNotAsErrors=1573 /p:WarningsNotAsErrors=1591"
• MSBuild simply overwrites properties if they are specified multiple times
• The intended and supported way for multiple parameters in MSBuild is a comma or semicolon separated list. Not all command line arguments support this, though.
  • the actual separator usage can be seen here. it is split by one of the valid separators and then further processed as a list
• Every parameter is unquoted before it is processed, so adding quotes around the parameter is fine

In total Cake's current handling for multiple parameters in the "MSBuildSettingsExtensions.WithProperty" method basically results in MSBuild discarding everything except the last parameter. As this definitely doesn't seem to be the intended behavior I would consider this a bug that should be fixed, even though it might change the behavior of old scripts that tried to use multiple parameters (but didn't actually).