embed funcr, custom With* function example

[pohly]

This allows custom loggers to provide their own additional methods
while using most of the implementation from funcr.

That funcr had to be extended to support the helper concept in
testing.T didn't feel right because fnlogger then always implemented
the CallStackHelperLogSink interface even when the helper was
nil. This gets fixed by moving the implementation of that into the
testing logger, using the new embedding mechanism of funcr.

As a drive-by fix, the testing logger no longer prints a redundant ":"
when the prefix is empty.

The logr package describes how to implement a custom With* function. A
runable example for that gets added in funcr/example_test.go.

[thockin]

I don't understand what's going on with FnLogSink being a distinct type. The first commit is what I expected. I'm not clear on the second.

[pohly]

FnLogSink is what custom LogSink embed into their own struct. fnlogger is the internal implementation of the LogSink interface which uses FnLogSink. These two must be distinct types because, for example, fnlogger.WithName must not be inherited by custom implementations because it wouldn't do the right thing (it copies only the struct it knows about, not the full struct of the custom implementation).

Let me add a runable example. Perhaps that'll clarify this.

[pohly]

I ended up refactoring funcr differently. It's now split into a formatter and the full LogSink implementation.

I also addressed one aspect of PR #60 that didn't quite feel right but couldn't be solved differently at that time: adding the helper function to funcr.Options was both a small API break (struct not comparable anymore), not quite the right place (not a formatting option) and had the effect that fnlogger always implemented the additional interface even when not needed. I reverted that change and moved the implementation of the helper handling entirely under testing.

The downside is that more boilerplate code is needed there.

The original purpose of this PR, providing a code example for a custom With* function, is served by the example stdout logger in funcr/example_test.go. The previous example in funcr itself got removed.

[pohly]

I think it would make sense to allow configuring the formatting, in particular the log level threshold. I've not added it to the PR to not bloat it further, but would like to see this addressed before logr v1.1.0.

The simplest solution would be:

func NewTestLoggerWithOptions(t *testing.T, opts funcr.Options) logr.Logger {
   ...
}

[thockin]

I'd rather see TestLogger get its own options struct, even if that is just a typedef to funcr.Options.

[pohly]

As a drive-by fix, the testing logger no longer prints a redundant ":" when the prefix is empty.

Output before:

$ go test -v ./testing
=== RUN   TestTestLogger
    test_test.go:28: : "level"=0 "msg"="info"
    test_test.go:29: : "level"=0 "msg"="V(0).info"
    test_test.go:31: : "msg"="error" "error"="error"
    test_test.go:32: : "level"=0 "msg"="hello world"
--- PASS: TestTestLogger (0.00s)

Output now:

=== RUN   TestLogger
    test_test.go:28: "level"=0 "msg"="info"
    test_test.go:29: "level"=0 "msg"="V(0).info"
    test_test.go:31: "msg"="error" "error"="error"
    test_test.go:32: testing: "level"=0 "msg"="with prefix"
    test_test.go:33: "level"=0 "msg"="hello world"
--- PASS: TestLogger (0.00s)

[pohly]

This comment needs to be updated...

[thockin]

s/method/function

[thockin]

Thanks!

/lgtm
/approve