Permit specifying that we should use utf-8 in ToolTask #6188
Conversation
There was a problem hiding this comment.
LGTM. This looks like the right approach for #6168 as well. We have it as up for grabs, so I'll leave that up to you if you want to add the ability to specify BOM usage "while we're here"™
|
WriteLinesToFile isn't a ToolTask, so although the fix would probably look almost identical, the code itself would be almost disjoint unless we wanted to move the ability to set encoding all the way back to TaskExtension, which doesn't really make sense. I'll leave it as-is, just noting that anyone working on that will hopefully see it referenced here and draw inspiration from this PR. |
| @@ -333,7 +333,6 @@ public partial class Exec : Microsoft.Build.Tasks.ToolTaskExtension | |||
| [Microsoft.Build.Framework.OutputAttribute] | |||
| public string StdOutEncoding { get { throw null; } set { } } | |||
| protected override string ToolName { get { throw null; } } | |||
| public string UseUtf8Encoding { get { throw null; } set { } } | |||
There was a problem hiding this comment.
Weird that the API addition in ToolTask does not show up.
There was a problem hiding this comment.
It's in M.B.Utilities.Core, so it's the next file. I was wondering if I should count this as a breaking change because someone could use UseUtf8Encoding for Exec without loading Utilities.Core, but I don't think there's anything that calls into it, so I think it's ok.
Forgind
added
the
merge-when-branch-open
Fixes #5724
Changes Made
Added UseUtf8Encoding parameter to ToolTask and removed it from Exec. Exec is a ToolTaskExtension, which is clearly an extension of ToolTask, but this may still count as a breaking change, in which case I could add it back to Exec with
newto make it compile.Testing
MrTrillian tried this with a reduced repro solution, and it resolved the problem.