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

Added QTextEdit and QPlainTextEdit sink #1986

Merged
merged 7 commits into from
Jun 28, 2021
Merged

Added QTextEdit and QPlainTextEdit sink #1986

merged 7 commits into from
Jun 28, 2021

Conversation

mguludag
Copy link
Contributor

Hello I've added custom sink for show logs in QTextEdit or QPlainTextEdit widget. Its only dependency is Qt.

mguludag added 2 commits June 28, 2021 23:13
QPlainTextEdit performs better than QTextEdit and its derivatives and also it has rich features
@gabime
Copy link
Owner

gabime commented Jun 28, 2021

Thanks. Could you please merge all to single file called “qt_sinks” ?

void sink_it_(const details::log_msg &msg) override {
memory_buf_t formatted;
base_sink<Mutex>::formatter_->format(msg, formatted);
auto str = std::string(formatted.begin(), formatted.end() - 2);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a way to avoid the expensive creation of std string? maybe use a string view?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok. I changed it to spdlog::string_view_t but widget's slots accepts QString so constructed QString and emit its signal.

void sink_it_(const details::log_msg &msg) override {
memory_buf_t formatted;
base_sink<Mutex>::formatter_->format(msg, formatted);
auto str = std::string(formatted.begin(), formatted.end() - 2);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same

@gabime gabime merged commit 5f4cc7b into gabime:v1.x Jun 28, 2021
@gabime
Copy link
Owner

gabime commented Jun 28, 2021

Thanks @mguludag

@FlorianWolters
Copy link

It took me some time to figure out how-to use qtextedit_sink_mt in qt_sinks.h with CMake v3.21.0 and spdlog v1.9.0.

The following minimal CMake example demonstrates how-to create the MOC generated C++ source file required for qt_sinks.h without using CMAKE_AUTOMOC:BOOL=TRUE. Using CMAKE_AUTOMOC could work in case spdlog is part of the CMake project itself and not an external "installation" by the means of find_package is used.

cmake_minimum_required(VERSION 3.21)

project(spdlog_qt_usage LANGUAGES CXX)

find_package(Qt5 REQUIRED CONFIG COMPONENTS Widgets)
find_package(spdlog REQUIRED CONFIG)

get_target_property(spdlog_include_dir spdlog::spdlog INTERFACE_INCLUDE_DIRECTORIES)
qt5_wrap_cpp(spdlog_qt_moc_source_files "${spdlog_include_dir}/spdlog/sinks/qt_sinks.h")

add_executable(main)
target_link_libraries(main
  PRIVATE
    Qt::Widgets
    spdlog::spdlog
  )
target_sources(main
  PRIVATE
    main.cpp
    ${spdlog_qt_moc_source_files}
  )

Questions:

  1. Are there any plans to simplify this?
  2. Is it really required that the "private" classes in qt_sinks.h inherit from QObject and use Q_OBJECT? What are the actual use cases for this? Shouldn't it work without the direct dependency to moc?

I think it would be great to have a solution in spdlog that doesn't require to run moc on a spdlog header file (though I think I am missing something here), e.g. refer to a solution like #1898, which doesn't use the class and macro.

@gabime
Copy link
Owner

gabime commented Jul 26, 2021

Are there any plans to simplify this?

A PR to simplify would be most welcome.

Is it really required that the "private" classes in qt_sinks.h inherit from QObject and use Q_OBJECT? What are the actual use cases for this? Shouldn't it work without the direct dependency to moc?

If you could find a way to simplify this as well, it would be great to get a PR.

@mguludag
Copy link
Contributor Author

mguludag commented Jul 26, 2021

It took me some time to figure out how-to use qtextedit_sink_mt in qt_sinks.h with CMake v3.21.0 and spdlog v1.9.0.

The following minimal CMake example demonstrates how-to create the MOC generated C++ source file required for qt_sinks.h without using CMAKE_AUTOMOC:BOOL=TRUE. Using CMAKE_AUTOMOC could work in case spdlog is part of the CMake project itself and not an external "installation" by the means of find_package is used.

cmake_minimum_required(VERSION 3.21)

project(spdlog_qt_usage LANGUAGES CXX)

find_package(Qt5 REQUIRED CONFIG COMPONENTS Widgets)
find_package(spdlog REQUIRED CONFIG)

get_target_property(spdlog_include_dir spdlog::spdlog INTERFACE_INCLUDE_DIRECTORIES)
qt5_wrap_cpp(spdlog_qt_moc_source_files "${spdlog_include_dir}/spdlog/sinks/qt_sinks.h")

add_executable(main)
target_link_libraries(main
  PRIVATE
    Qt::Widgets
    spdlog::spdlog
  )
target_sources(main
  PRIVATE
    main.cpp
    ${spdlog_qt_moc_source_files}
  )

Questions:

  1. Are there any plans to simplify this?
  2. Is it really required that the "private" classes in qt_sinks.h inherit from QObject and use Q_OBJECT? What are the actual use cases for this? Shouldn't it work without the direct dependency to moc?

I think it would be great to have a solution in spdlog that doesn't require to run moc on a spdlog header file (though I think I am missing something here), e.g. refer to a solution like #1898, which doesn't use the class and macro.

Thanks for the idea. I will test a simple solution and PR :)

Status 7/2/21 :
I tried solutions and methods like #1898 and It works in GUI thread as expected. But due the Qt's design direct access GUI elements from different thread crashes the application even with mutex 🤔

@mguludag mguludag mentioned this pull request Jul 27, 2021
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

Successfully merging this pull request may close these issues.

3 participants