GH1106: Multiple values passed to WithProperty are overridden - #2110
mschlaffer wants to merge 3 commits into
Conversation
|
|
770732d to
95e2547
Compare
augustoproiete
changed the title
95e2547 to
0a5aa03
Compare
…mma separated list
0a5aa03 to
6f748a0
Compare
pascalberger
left a comment
pascalberger
left a comment
There was a problem hiding this comment.
@caopi Thanks you for your contribution and sorry for the late answer. We're now at a point where we can change this behavior in an upcoming release.
We would prefer to use semicolon instead of comas for separating the values as mentioned in the comment. Additionally there should be some integration tests for this, which makes sure that this actually works together with MsBuild.
Are you still in a position to continue with this PR or prefer for someone else to take over?
Co-authored-by: Pascal Berger <pascal.berger@gmail.com>
|
Hey Pascal, sorry for the late reply as well. I've quickly accepted your suggestion of course, but I would prefer if someone else could take over the integration test part. Haven't worked with C# and Cake in a while nowadays and don't have a lot of time to commit to this right now, sorry! |
gep13
changed the title
paulomorgado
left a comment
paulomorgado
left a comment
There was a problem hiding this comment.
Can tests be added that have MSBuild escaped characters?
| } | ||
| else if (properties[propertyKey].Count == 1) | ||
| { | ||
| yield return string.Concat("/p:", propertyKey, "=", properties[propertyKey].First().EscapeMSBuildPropertySpecialCharacters()); |
There was a problem hiding this comment.
Consider using string interpolation for readbility instead:
| yield return string.Concat("/p:", propertyKey, "=", properties[propertyKey].First().EscapeMSBuildPropertySpecialCharacters()); | |
| yield return $"/p:{propertyKey}={properties[propertyKey].First().EscapeMSBuildPropertySpecialCharacters())}"; |
| if (properties[propertyKey].Count > 1) | ||
| { | ||
| yield return string.Concat("/p:", propertyKey, "=", propertyValue.EscapeMSBuildPropertySpecialCharacters()); | ||
| var commaSeparatedValues = string.Join(';', properties[propertyKey].Select(x => x.EscapeMSBuildPropertySpecialCharacters())); |
There was a problem hiding this comment.
Consider using string interpolation for readbility instead:
| var commaSeparatedValues = string.Join(';', properties[propertyKey].Select(x => x.EscapeMSBuildPropertySpecialCharacters())); | |
| var commaSeparatedValues = $";{properties[propertyKey].Select(x => x.EscapeMSBuildPropertySpecialCharacters()))}"; |
| { | ||
| yield return string.Concat("/p:", propertyKey, "=", propertyValue.EscapeMSBuildPropertySpecialCharacters()); | ||
| var commaSeparatedValues = string.Join(';', properties[propertyKey].Select(x => x.EscapeMSBuildPropertySpecialCharacters())); | ||
| yield return string.Concat("/p:", propertyKey, "=", '"', commaSeparatedValues, '"'); |
There was a problem hiding this comment.
Consider using string interpolation for readbility instead:
| yield return string.Concat("/p:", propertyKey, "=", '"', commaSeparatedValues, '"'); | |
| yield return $"/p:{propertyKey}=\"{commaSeparatedValues}\""; |
This branch has not been deployed
7 participants
Changed MSBuildExtension's WithProperty to forward multiple parameters as a comma separated list surrounded by quotes. Single parameter behavior is unchanged to before and does not have the added quotes.
This should fix #1106. Check the comments there for more details.