Closed Zhentar closed 5 years ago
Thank you for contributing!
I feel sort of ambivalent about interpolated strings in general, but this looks like a reasonable change. I confess I only have a rudimentary understanding of how the FormattableString
stuff works (the compiler recognizes those types/interfaces?), so when I say it "looks okay to me", I admit to not giving it super careful scrutiny.
The main important thing I think this change is missing is some documentation--it's probably not a very discoverable feature. Could you add something about it to doc/Color.md?
There are also a few formatting inconsistencies (like no space after comment starter //comment
), but if you don't get them all, I can fix them in a followup. Thanks for the .editorconfig stuff, btw!
Updates made.
FormattableString is basically just a fancy String.Format wrapper. The example code I posted above compiles to this:
ColorString testString = new ColorString(FormattableStringFactory.Create("Testing {0:X4,Red,Blue} {1}", new object[] { 123, new ColorString(ConsoleColor.Yellow, "testing!")}));
This one is easier to show than tell:
The one unfortunate aspect of these changes is that the C# (and VB.NET) compiler always favors
string
overloads overFormattableString
for interpolated strings, so to keep the API consistent I had to remove the publicstring
overloads for the constructor, Append, and AppendLine, instead relying on the implicit conversion of string to ColorString, so it makes consuming the API slightly more confusing and slightly less efficient for strings (since there is an unnecessary intermediate ColorString allocation). But on the up side, there should be a pretty significant efficiency gain when using interpolated strings or AppendFormat (because the string.Format becomes lazy and shares the ColorString's StringBuilder).