fix: support `ICustomFormatter` via user-supplied `IFormatProvider`

[aradalvand]

Fixes #186

I don't believe this is a breaking change. Because the hand-written if clauses and whatnot that were previously there were doing, in my analysis, effectively a strict subset of what ScalarValue.Render does internally (and the latter does more, which is the crucial fix here, see #186).

P.S. There are two other approaches that you can see in the first and second commits. But I think delegating to ScalarValue.Render is the most correct one.

[nblumhardt]

Hi! Thanks for digging into this.

ScalarValue.Render() will (IIRC) lead to "quoted string"-style output and non-JSON structured data; a more tactical change might be needed here.

Would also be worth including a test to show where the behavior will differ with a custom formatter; there's a similar test in https://github.com/serilog/serilog/blob/dev/test/Serilog.Tests/Formatting/Display/MessageTemplateTextFormatterTests.cs#L168 (though it's a bit lean/minimal :-)).

[aradalvand]

Hi @nblumhardt! Thanks for checking this out!

ScalarValue.Render() will (IIRC) lead to "quoted string"-style output

You're right, only if the parameter is of type string though, which I think makes sense. Are you saying that's problematic? َUpdate: Ended up special-casing that.

non-JSON structured data

I think that's solved by virtue of the fact that we are still special-casing non-scalars and JSON-serializing them:

Would also be worth including a test to show where the behavior will differ with a custom formatter

Good point; I'll add a test for this.

[aradalvand]

I just added a couple of test cases (for testing CleanMessageTemplateFormatter in an isolated fashion as well as an "end-to-end" test), verifying that the ICustomFormatter works.

I also special-cased strings to make them unquoted, like before, as you seemed to prefer (and I think I agree).

Either way, I didn't touch the old tests, and all the tests (+ the new ones) are now passing, including the JSON test.

[nblumhardt]

Thanks for following up! Sorry about the slow reply.

Formatting through ScalarValue is something of a legacy path, so just making the minimal change here to support ICustomFormatter would be preferred. I can see that the existing code could use a bit of sanity-checking and clean-up :-) but I can take care of that down the path. The minimal change to support the added functionality would be a quicker and easier review/merge. Thanks again!

[aradalvand]

Hi @nblumhardt — no worries! I just made another commit; let me know if the changeset is ideal now (no longer forwarding to ScalarValue).