Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Expose formatter to public or protected in CommonWriter #360

Open
c0nstexpr opened this issue Jul 25, 2023 · 4 comments
Open

Expose formatter to public or protected in CommonWriter #360

c0nstexpr opened this issue Jul 25, 2023 · 4 comments

Comments

@c0nstexpr
Copy link

c0nstexpr commented Jul 25, 2023

open class CommonWriter(private val messageStringFormatter: MessageStringFormatter = DefaultFormatter) : LogWriter() {

At now CommonWriter's formatter is private, which make it impossible to use custom formatter when inheriting writer. See below

class MyWriter() : CommonWriter(MyFormatter()) {
    override fun log(severity: Severity, message: String, tag: String, throwable: Throwable?) =
        // Cannot access 'messageStringFormatter': it is invisible (private in a supertype)
        Mylog.log(messageStringFormatter.formatMessage(severity, Tag(tag), Message(message)))
}
@findjigar
Copy link
Contributor

It's possible to customize Message Formatting. Did you check out this docs? https://kermit.touchlab.co/docs/configuration/MESSAGE_FORMATTING

@c0nstexpr
Copy link
Author

It's possible to customize Message Formatting. Did you check out this docs? https://kermit.touchlab.co/docs/configuration/MESSAGE_FORMATTING

Yes, I know the current way to customize log writer. And I also successfully implement my own writer.
But if we implement a class to inherit CommonWriter, providing customization by overriding base log method seems to be pointless. Because the inheritor cannot access the formatter.

@kpgalligan
Copy link
Collaborator

Sorry for the long delay. Not sure the original poster cares at this point, but I'm cleaning up the issues list. Lots of them perpetually stay in the "we'll look at this when we get some extra time", but that never happens on its own.

The idea of MessageStringFormatter was to provide a way to customize formatting without needing to write custom writers. If you were going to create a custom writer, you'd either be doing it because you wanted more control over formatting, or you'd need to take the MessageStringFormatter instance as a constructor arg and could just hold onto it in your custom code. But, I can see the counter argument.

I'm fairly reluctant to expand the API too much, but this might be worth it. I'd just want to think through it a bit.

@c0nstexpr
Copy link
Author

Sorry for the long delay. Not sure the original poster cares at this point, but I'm cleaning up the issues list. Lots of them perpetually stay in the "we'll look at this when we get some extra time", but that never happens on its own.

The idea of MessageStringFormatter was to provide a way to customize formatting without needing to write custom writers. If you were going to create a custom writer, you'd either be doing it because you wanted more control over formatting, or you'd need to take the MessageStringFormatter instance as a constructor arg and could just hold onto it in your custom code. But, I can see the counter argument.

I'm fairly reluctant to expand the API too much, but this might be worth it. I'd just want to think through it a bit.

Yes, it is all about api design.
My first idea to implement my own logger, is to check whether library has provide a good way for me to do it.
Then I see the CommonWriter qualified by open. And I'm more confused by the private member in ctor.

Originally, I would write something like

class MyWriter() : CommonWriter(MyFormatter()) {
    override fun log(severity: Severity, message: String, tag: String, throwable: Throwable?) {
        client.msg(
            coloredText(
                levelFmt.getOrElse(severity) { severity.defaultFmt() },
                messageStringFormatter.formatMessage(severity, Tag(tag), Message(message)),
            ),
        )
    }
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants