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

Non-default channel logs get redirected into the default channel #31

Open
barsoosayque opened this issue Sep 12, 2018 · 2 comments
Open

Comments

@barsoosayque
Copy link
Contributor

Consider the following example:

Log::SetLevel(LogLevel::Verbose);

Log::Connect("rare", [](const LogEntry& entry) {
    std::printf("[rare] %s\n", entry.Message.c_str());
});
Log::Connect("legendary", [](const LogEntry& entry) {
    std::printf("[legendary] %s\n", entry.Message.c_str());
});
Log::Connect([](const LogEntry& entry) {
    std::printf("[common] %s\n", entry.Message.c_str());
});

Log::Verbose("rare", "blue message");
Log::Verbose("legendary", "gold message");
Log::Verbose("gray message");

Expected output is:

[rare] blue message
[legendary] gold message
[common] gray message

But what I get is:

[common] blue message
[rare] blue message
[common] gold message
[legendary] gold message
[common] gray message

So, it seems that all log entries from non-default channels fires the default channel signal.

(Also, I'm not sure why, but the default channel is not Verbose by default, whereas others are)

@mogemimi
Copy link
Owner

Hi, thank you for feedback! 👍 You've got a point. This is the intended behaviour and the default channel receives all messages included tagged logs, but it confuses new users.

Also, the verbosity level of the default channel is set to LogLevel::Internal in the main function.

The first idea which comes to mind is using early return by tags.

Log::Connect("rare", [](const LogEntry& entry) {
    std::printf("[rare] %s\n", entry.Message.c_str());
});
Log::Connect("legendary", [](const LogEntry& entry) {
    std::printf("[legendary] %s\n", entry.Message.c_str());
});
Log::Connect([](const LogEntry& entry) {
    if (!entry.Tag.empty()) {
        return;
    }
    std::printf("[common] %s\n", entry.Message.c_str());
});

The second idea makes it explicitly specify an anonymous channel by using new LogChannels enum class.

enum class LogChannels {
    // Represents a default channel without a tag.
    Default,

    // Broadcasts all log messages ignoring tags.
    All,
};

class POMDOG_EXPORT Log final {
public:
    // This function is the same as `Connect(LogChannels::Default, slot)`.
    static Connection Connect(const std::function<void(const LogEntry&)>& slot);

    // This function can explicitly specify the channel by name.
    static Connection Connect(const std::string& channelName, const std::function<void(const LogEntry&)>& slot);

    // The following function is a new API, it can specify `enum class LogChannels`
    static Connection Connect(LogChannels channel, const std::function<void(const LogEntry&)>& slot);
};

What do you think about it or do you have any idea? @barsoosayque

@barsoosayque
Copy link
Contributor Author

barsoosayque commented Sep 17, 2018

The first idea which comes to mind is using early return by tags.

I don't think that it is appropriate way to handle this since translation fires the signal, invoking lambda just to close itself. It's seems like a good workaround, but there should be a better way in the library to adjust logging.

The second idea makes it explicitly specify an anonymous channel by using new LogChannels enum class.

I think this is a much more better way, but an enum is sort of complex solution for that. I consider the following is just enough to be effective and verbose about it's behavior:

// Default channel
static Connection Connect(const std::function<void(const LogEntry&)>& slot);

// Specific channel + no redirection to default channel
static Connection Connect(const std::string& channelName, 
                          const std::function<void(const LogEntry&)>& slot);

// Specific channel + optional redirection to default channel
static Connection Connect(const std::string& channelName, 
                          bool forwardToDefault, 
                          const std::function<void(const LogEntry&)>& slot);

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

2 participants