feat: complete launchpad logging integration#753
Open
Ivy233 wants to merge 1 commit intolinuxdeepin:masterfrom
Open
feat: complete launchpad logging integration#753Ivy233 wants to merge 1 commit intolinuxdeepin:masterfrom
Ivy233 wants to merge 1 commit intolinuxdeepin:masterfrom
Conversation
Reviewer's GuideIntegrates optional dde-api based event logging into the launchpad shared launcher controller, conditionally wiring it via CMake detection of dde-api headers and aligning Debian build metadata with the new logging capability. Sequence diagram for launchpad startup logging in LauncherController constructorsequenceDiagram
participant CMake as CMake_build
participant LC as LauncherController
participant Settings as QSettings
participant Log as QLoggingCategory_logController
participant EventLogger as DDE_EventLogger_EventLogger
CMake->>CMake: detect dde-api/eventlogger.hpp
alt eventlogger header found
CMake->>LC: define HAVE_DDE_API_EVENTLOGGER
else not found
CMake-->>LC: HAVE_DDE_API_EVENTLOGGER not defined
end
LC->>Settings: value("current_frame", "WindowedFrame")
Settings-->>LC: m_currentFrame
LC->>Log: qCInfo logController << "Current frame mode:" << m_currentFrame
alt HAVE_DDE_API_EVENTLOGGER defined
LC->>EventLogger: instance().init("org.deepin.dde.launchpad", false)
LC->>EventLogger: instance().writeEventLog(EventLoggerData(EVENT_LOGGER_LAUNCHPAD_MODE, "launchpad_config", {"launchpad_mode", m_currentFrame}))
LC->>Log: qCInfo logController << "EventLogger: launchpad mode on startup:" << m_currentFrame
else event logging disabled
LC-->>LC: skip event logging
end
Sequence diagram for logging on frame mode change via setCurrentFramesequenceDiagram
actor User
participant UI as Launchpad_UI
participant LC as LauncherController
participant Log as QLoggingCategory_logController
participant EventLogger as DDE_EventLogger_EventLogger
User->>UI: change launchpad frame mode
UI->>LC: setCurrentFrame(newFrame)
LC->>LC: m_currentFrame = newFrame
LC->>Log: qDebug << "set current frame:" << m_currentFrame
alt HAVE_DDE_API_EVENTLOGGER defined
LC->>EventLogger: instance().writeEventLog(EventLoggerData(EVENT_LOGGER_LAUNCHPAD_MODE, "launchpad_config", {"launchpad_mode", m_currentFrame}))
LC->>Log: qCInfo logController << "EventLogger: launchpad mode changed to:" << m_currentFrame
else event logging disabled
LC-->>LC: no event log emitted
end
LC->>LC: m_pendingHide = false
LC->>LC: m_timer.start()
LC-->>UI: emit currentFrameChanged()
Class diagram for optional EventLogger integration in LauncherControllerclassDiagram
class LauncherController {
- QString m_currentFrame
- bool m_pendingHide
+ LauncherController(QObject *parent)
+ void setCurrentFrame(QString frame)
}
class DDE_EventLogger_EventLogger {
+ static DDE_EventLogger_EventLogger instance()
+ void init(QString appId, bool enableDebug)
+ void writeEventLog(DDE_EventLogger_EventLoggerData data)
}
class DDE_EventLogger_EventLoggerData {
+ DDE_EventLogger_EventLoggerData(qint64 eventId, QString category, std::map<QString, QString> payload)
+ qint64 eventId
+ QString category
+ std::map<QString, QString> payload
}
class BuildConfig {
+ bool HAVE_DDE_API_EVENTLOGGER
+ qint64 EVENT_LOGGER_LAUNCHPAD_MODE
}
LauncherController ..> DDE_EventLogger_EventLogger : uses when
LauncherController ..> DDE_EventLogger_EventLoggerData : creates when
BuildConfig <.. LauncherController : compile_time_flag
BuildConfig <.. DDE_EventLogger_EventLogger : event_id_constant
Flow diagram for CMake-based optional dde-api EventLogger wiringflowchart TD
A[CMake_configuration_start] --> B[unset DDE_API_EVENTLOGGER_INCLUDE_DIR CACHE]
B --> C[find_path for dde-api/eventlogger.hpp in /usr/include]
C -->|found| D[set HAVE_DDE_API_EVENTLOGGER ON]
C -->|not_found| E[HAVE_DDE_API_EVENTLOGGER is OFF]
D --> F[message: Found dde-api eventlogger.hpp]
E --> G[message: event logging will be disabled]
D --> H[add compile_definition HAVE_DDE_API_EVENTLOGGER to target launchpadcommon]
E --> I[no compile_definition added]
H --> J[LauncherController compiled with EventLogger calls enabled]
I --> K[LauncherController compiled without EventLogger calls]
J --> L[runtime: launchpad emits dde-api event logs]
K --> M[runtime: only standard logging used]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- The CMake detection for
dde-api/eventlogger.hppis hardcoded to/usr/include; consider using standard include search paths (e.g., relying onCMAKE_INCLUDE_PATH/CMAKE_PREFIX_PATHor a pkg-config module) instead of a fixed path to make the build more portable. - The event logging calls for startup and frame changes duplicate the construction of
EventLoggerData; extracting a small helper method (e.g.,logLaunchpadMode(const QString &mode)) would reduce repetition and keep the logging logic consistent in one place.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The CMake detection for `dde-api/eventlogger.hpp` is hardcoded to `/usr/include`; consider using standard include search paths (e.g., relying on `CMAKE_INCLUDE_PATH`/`CMAKE_PREFIX_PATH` or a pkg-config module) instead of a fixed path to make the build more portable.
- The event logging calls for startup and frame changes duplicate the construction of `EventLoggerData`; extracting a small helper method (e.g., `logLaunchpadMode(const QString &mode)`) would reduce repetition and keep the logging logic consistent in one place.
## Individual Comments
### Comment 1
<location path="launchercontroller.cpp" line_range="50-55" />
<code_context>
qCInfo(logController) << "Current frame mode:" << m_currentFrame;
+#ifdef HAVE_DDE_API_EVENTLOGGER
+ DDE_EventLogger::EventLogger::instance().init("org.deepin.dde.launchpad", false);
+ DDE_EventLogger::EventLogger::instance().writeEventLog(
+ DDE_EventLogger::EventLoggerData(EVENT_LOGGER_LAUNCHPAD_MODE, "launchpad_config", {
+ {"launchpad_mode", m_currentFrame}
+ }));
+ qCInfo(logController) << "EventLogger: launchpad mode on startup:" << m_currentFrame;
+#endif
+
</code_context>
<issue_to_address>
**suggestion:** Consider wrapping the event logging logic in a small helper to avoid duplication and keep constructor/setter in sync.
The `writeEventLog` call with `EVENT_LOGGER_LAUNCHPAD_MODE` and identical payload appears both here and in `setCurrentFrame`. Extracting this into a small private helper (e.g. `logLaunchpadMode(const QString &mode, const QString &reason)`) would remove duplication and ensure both call sites stay consistent when the event format or ID changes.
Suggested implementation:
```cpp
#include <private/qguiapplication_p.h>
```
```cpp
#include <QDBusConnection>
#include <QLoggingCategory>
#ifdef HAVE_DDE_API_EVENTLOGGER
#include <dde-api/eventlogger.hpp>
// Event ID for launchpad mode (10-digit number)
constexpr qint64 EVENT_LOGGER_LAUNCHPAD_MODE = 1000600016;
namespace {
void logLaunchpadMode(const QString &mode, const QString &reason)
{
auto &logger = DDE_EventLogger::EventLogger::instance();
logger.init(QStringLiteral("org.deepin.dde.launchpad"), false);
logger.writeEventLog(DDE_EventLogger::EventLoggerData(
EVENT_LOGGER_LAUNCHPAD_MODE,
reason,
{
{ QStringLiteral("launchpad_mode"), mode }
}));
qCInfo(logController) << "EventLogger: launchpad mode" << reason << ":" << mode;
}
}
#endif
```
```cpp
m_currentFrame = settings.value("current_frame", "WindowedFrame").toString();
qCInfo(logController) << "Current frame mode:" << m_currentFrame;
#ifdef HAVE_DDE_API_EVENTLOGGER
logLaunchpadMode(m_currentFrame, QStringLiteral("launchpad_config"));
#endif
```
I only see the constructor snippet. To fully apply the refactoring and avoid duplication, you should also:
1. Locate the `setCurrentFrame(...)` implementation in `launchercontroller.cpp` and replace its duplicated `DDE_EventLogger::EventLogger::instance().init(...)` / `writeEventLog(...)` block (and related `qCInfo` line) with:
```cpp
#ifdef HAVE_DDE_API_EVENTLOGGER
logLaunchpadMode(m_currentFrame, QStringLiteral("launchpad_config"));
#endif
```
2. Ensure there is no remaining direct use of `EVENT_LOGGER_LAUNCHPAD_MODE` outside `logLaunchpadMode` (unless intentionally needed elsewhere), so future changes only need to touch the helper.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Ivy233
force-pushed
the
feat/event-logger-integration
branch
from
0271567 to
35baf43
Compare
Ivy233
requested review from
18202781743,
BLumia,
mhduiy,
robertkill,
wjyrich and
yixinshark
BLumia
previously approved these changes
Apr 27, 2026
Ivy233
removed request for
18202781743,
mhduiy,
robertkill,
wjyrich and
yixinshark
wineee
reviewed
Apr 27, 2026
wineee
reviewed
Apr 27, 2026
Ivy233
force-pushed
the
feat/event-logger-integration
branch
3 times, most recently
from
b9e343c to
36d9123
Compare
使用多字段事件日志记录器并迁移到 cmake find_package - Migrate from find_path to find_package(DDEAPI) - Link DDEAPI::EventLogger to dde-control-center - Fix pt_BR translation for control center name PMS: TASK-388657
Ivy233
force-pushed
the
feat/event-logger-integration
branch
from
36d9123 to
33fbf9a
Compare
deepin pr auto review这段代码的主要目的是为启动器添加事件日志记录功能，通过集成 1. 语法逻辑
2. 代码质量
3. 代码性能
4. 代码安全
综合改进建议代码示例针对 // ... 头文件包含 ...
namespace {
Q_LOGGING_CATEGORY(logController, "org.deepin.dde.launchpad.controller")
// 定义常量，避免魔法数字和硬编码字符串
namespace EventLogConstants {
constexpr qint64 EVENT_ID_LAUNCHPAD_MODE = 1000610012; // TODO: 添加该ID来源的注释
constexpr const char* EVENT_SOURCE = "launchpad_config";
constexpr const char* KEY_MODE = "launchpad_mode";
constexpr int MAX_MODE_LENGTH = 64; // 假设模式名称不应超过64个字符
}
void logLaunchpadMode(const QString &mode, const char *description)
{
// 安全性改进：输入验证
if (mode.isEmpty()) {
qCWarning(logController) << "Attempted to log an empty launchpad mode";
return;
}
// 可选：防止过长的字符串
if (mode.length() > EventLogConstants::MAX_MODE_LENGTH) {
qCWarning(logController) << "Launchpad mode exceeds maximum length";
return;
}
#ifdef HAVE_DDE_API_EVENTLOGGER
// 使用定义的常量
DDE_EventLogger::EventLogger::instance().writeEventLog(
DDE_EventLogger::EventLoggerData(EventLogConstants::EVENT_ID_LAUNCHPAD_MODE,
QLatin1String(EventLogConstants::EVENT_SOURCE),
{
{QLatin1String(EventLogConstants::KEY_MODE), mode}
}));
#endif
// 使用 QLatin1String 或 QStringLiteral 提高字符串字面量处理效率
qCInfo(logController) << "EventLogger: launchpad mode" << description << ":" << mode;
}
// ...总结这段代码整体质量良好，逻辑清晰，正确使用了 CMake 的特性来处理可选依赖。主要的改进点在于：
这些改进将有助于代码的长期维护和可读性。 |
18202781743
approved these changes
Apr 29, 2026
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: 18202781743, Ivy233 The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Add optional dde-api detection and Debian build dependency updates, and finish launchpad-side logging support in the shared launcher controller. Keep the launchpad build configuration and packaging aligned with the new logging integration.
feat: 完善启动器日志集成
增加可选 dde-api 检测与 Debian 构建依赖更新，并在共享启动器控制器中补全启动器侧日志支持。 同步更新启动器的构建配置与打包依赖，使新的日志集成能够正确构建和交付。
PMS: TASK-388657
Summary by Sourcery
Integrate optional dde-api based event logging for launchpad mode and wire it into the shared launcher controller while updating build configuration accordingly.
New Features:
Enhancements:
Build: