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

Logging facade #85

Closed
ratijas opened this issue Apr 30, 2020 · 6 comments
Closed

Logging facade #85

ratijas opened this issue Apr 30, 2020 · 6 comments
Labels
A-wrappers Area: Wrappers and bindings C-feature-request Category: A feature request, i.e: not implemented / a PR.

Comments

@ratijas
Copy link
Collaborator

ratijas commented Apr 30, 2020

Not sure is it just me, or the logging functionality does not work from Qt side by default. I had to copy the handler function from qmetaobject tests, and explicitly install with install_message_handler function.

Now, given that there is a message type enum QtMsgType {QtDebugMsg, QtWarningMsg, ...} it should be easy to add a function which forwards Qt's messages to the rust logging facade's info!, warn!, ....

@ratijas
Copy link
Collaborator Author

ratijas commented Apr 30, 2020

Since this is going to be a pass-through system, it is crucial to retain as much context as possible.

log! has a notion of "target" — tag / name / context / module / whatever prefix to figure out where this line is coming from.

Fortunately, there is a notion of a "category" in Qt's logging system, implemented as const char * string, and wrapped together with severity (level) in a QLoggingCategory class.

When exposed through qInstallMessageHandler's QtMessageHandler callback signature, category can be retrieved via QMessageLogContext::category member field. And there are already wrappers for that in qmetaobject! They even take care on null values. The only thing is, according to Qt source code, default value for a category is (surprisingly) a "default" string, not an empty one.

@ratijas
Copy link
Collaborator Author

ratijas commented Apr 30, 2020

We've had a big discussion on Telegram Rust group regarding lifetimes, arguments formatting and problems related to format_args! macro, and came to the conclusion that match with single all-capturing arm is a common trick to allow retention of temporal objects for "a little bit longer". So, my final version looks like this:

use log::{Level, logger, Record};
use qmetaobject::*;
use qmetaobject::QtMsgType::*;

fn map_level(lvl: &QtMsgType) -> Level {
    match lvl {
        QtDebugMsg => Level::Debug,
        QtWarningMsg => Level::Warn,
        QtCriticalMsg => Level::Error,
        QtFatalMsg => Level::Error,
        QtInfoMsg => Level::Info,
    }
}

extern "C" fn log_capture(msg_type: QtMsgType,
                          context: &QMessageLogContext,
                          message: &QString) {
    let level = map_level(&msg_type);
    let target = match context.category() {
        "" => "default",
        x => x,
    };
    let file = match context.file() {
        "" => None,
        x => Some(x),
    };
    let line = match context.line() {
        // line numbers start from 1, right?
        0 => None,
        x => Some(x as _),
    };
    let mut record = Record::builder();
    record.level(level)
        .target(target)
        .file(file)
        .line(line)
        .module_path(None);
    match context.function() {
        "" => match format_args!("{}", message) {
            args => logger().log(&record.args(args).build()),
        },
        f => match format_args!("[in {}] {}", f, message) {
            args => logger().log(&record.args(args).build()),
        }
    }
}

Note that most context data is retained as first-class members of rust logging facade, except for context->function which is included in formatted arguments as-is.

@ogoffart
Copy link
Member

Sounds like a good idea that could be put behind a feature

@ratijas
Copy link
Collaborator Author

ratijas commented Apr 30, 2020

behind a feature

Exactly what I thought as well! The only other option is a separate micro-crate, but that's just too much of a left-pad, and also heavily depends on core qmetaobject-rs anyway.

@ratijas
Copy link
Collaborator Author

ratijas commented Apr 30, 2020

On the second thought, log facade is very lightweight, is feature-gate really needed?

Logger cannot register itself upon startup, user has to call initialization routine manually anyway. So having feature to prevent auto-registration is not the case, because there is no auto-registration possible in rust.

@ratijas ratijas closed this as completed May 4, 2020
@ratijas
Copy link
Collaborator Author

ratijas commented May 4, 2020

Implemented in #86

@ratijas ratijas added A-wrappers Area: Wrappers and bindings C-feature-request Category: A feature request, i.e: not implemented / a PR. labels Jun 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-wrappers Area: Wrappers and bindings C-feature-request Category: A feature request, i.e: not implemented / a PR.
Projects
None yet
Development

No branches or pull requests

2 participants