-
Notifications
You must be signed in to change notification settings - Fork 82
cxx-qt-lib: Add two more QtLogging functions #1227
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
base: main
Are you sure you want to change the base?
Conversation
9772397
to
7c5ef0b
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1227 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 73 73
Lines 12681 12681
=========================================
Hits 12681 12681 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some smaller issues, but more importantly we need to investigate whether set_message_pattern is safe to call!
|
||
/// Generates a formatted string out of the type, context, str arguments. | ||
#[cxx_name = "qSetMessagePattern"] | ||
fn set_message_pattern(pattern: &QString); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The Qt documentation states that qFormatLogMessage is thread-safe.
However, it does not state that set_message_pattern
is thread-safe!
Can you investigate whether this function is thread-safe?
Otherwise we may need to make it unsafe
or enforce safety somehow...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note that we're having similar issues with #1205
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's fine to mark it as unsafe, I should probably add a comment explaining why it is right?
Ping @redstrate, can you solve the remaining issues? |
b24934b
to
337c4e8
Compare
Done, sorry it's been a busy week - but I have time now :) |
337c4e8
to
0b04610
Compare
This adds two new global functions available in <QtLogging>: qFormatLogMessage and qSetMessagePattern. qFormatLogMessage is similar to qt_message_output, but actually documented. Instead of going to the default message handler though, it outputs to a QString. This is probably not useful without message handler support, but it's easy to expose. qSetMessagePattern is very useful, as it allows CXX-Qt applications to set custom Qt message patterns for logging.
0b04610
to
6c13f1e
Compare
This adds two new global functions available in : qFormatLogMessage and qSetMessagePattern.
qFormatLogMessage is similar to qt_message_output, but actually documented. Instead of going to the default message handler though, it outputs to a QString. This is probably not useful without message handler support, but it's easy to expose.
qSetMessagePattern is very useful, as it allows CXX-Qt applications to set custom Qt message patterns for logging.