-
Notifications
You must be signed in to change notification settings - Fork 4.6k
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
Simplified Qt sinks #2016
Simplified Qt sinks #2016
Conversation
Removed private class that derived from QObject
Removed private class that derived from QObject
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.
trim newline chars instead of remove 2 chars
include/spdlog/sinks/qt_sinks.h
Outdated
template <typename Mutex> | ||
class qtextedit_sink : public base_sink<Mutex> { | ||
public: | ||
qtextedit_sink(QTextEdit *textedit = nullptr) { |
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.
why allow nullptr by default if it throws if nullptr?
include/spdlog/sinks/qt_sinks.h
Outdated
~qtextedit_sink() { flush_(); } | ||
|
||
protected: | ||
void sink_it_(const details::log_msg &msg) override { |
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.
can never be null. why check it?
include/spdlog/sinks/qt_sinks.h
Outdated
void flush_() override {} | ||
|
||
private: | ||
QTextEdit* qtextedit = nullptr; |
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.
please rename to qtextedit_ like rest of spdlog
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.
removed nullptr checks and renamed member vars
Thanks. Please fix the conflict so i can merge |
base_sink<Mutex>::formatter_->format(msg, formatted); | ||
string_view_t str = string_view_t(formatted.data(), formatted.size()); | ||
QMetaObject::invokeMethod(qtextedit_,"append", Qt::AutoConnection, Q_ARG(QString, QString::fromUtf8(str.data(), static_cast<int>(str.size())).trimmed())); | ||
} |
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.
why the change from the simple append()? seems inefficient.
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.
Direct calling GUI widget functions from another threads in Qt app results crash. Qt forces us to use their own Q_OBJECT macro and connect function or QMetaObject::invokemethod function.
Thanks @mguludag |
Thanks @FlorianWolters for this idea. New header is more simpler and no private classes derived from QObject. Its also thread-safe (logging from any thread is safe)