refactor(notification): move wayland guard into initScreenLockedState#1612
Merged
Conversation
Reviewer's guide (collapsed on small PRs)Reviewer's GuideRefactors Wayland platform detection for notification screen lock handling by moving the guard into initScreenLockedState and switching to DGuiApplicationHelper’s Wayland attribute check, while adding a TODO for future Wayland lock support. Flow diagram for initScreenLockedState Wayland guard refactorflowchart TD
A[NotificationManager constructor] --> B[initScreenLockedState]
B --> C{"DGuiApplicationHelper::testAttribute(IsWaylandPlatform)"}
C -- true --> D[return]
C -- false --> E[Connect to org.deepin.dde.LockFront1<br/>and initialize screen lock state]
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 left some high level feedback:
- Consider updating the TODO comment text to clearly state the intention (e.g. "TODO: implement screen lock detection in Wayland sessions") to avoid confusion about the current behavior.
- When early-returning for Wayland in initScreenLockedState, you might want to add a debug log or comment explaining that screen lock state is intentionally not initialized on Wayland yet, so future readers understand why this path is skipped.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider updating the TODO comment text to clearly state the intention (e.g. "TODO: implement screen lock detection in Wayland sessions") to avoid confusion about the current behavior.
- When early-returning for Wayland in initScreenLockedState, you might want to add a debug log or comment explaining that screen lock state is intentionally not initialized on Wayland yet, so future readers understand why this path is skipped.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
18202781743
previously approved these changes
May 29, 2026
mhduiy
force-pushed
the
test
branch
5 times, most recently
from
8059512 to
1c7a444
Compare
18202781743
approved these changes
May 29, 2026
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: 18202781743, mhduiy 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 |
1. Added fallback condition to call initScreenLockedState() when platform name is non-empty, ensuring the function runs on Wayland as well 2. Previously skipped on Wayland only; now also handles empty platform name case that occurs in unit test environments 3. Marks intention for subsequent migration to the login1 interface Log: Expand screen lock initialization condition to work in unit test and Wayland environments fix(notification): 扩展屏幕锁定初始化条件以支持单元测试 1. 新增 platformName 非空时的回退条件，确保在 Wayland 上也能调用 initScreenLockedState() 2. 原先仅在 Wayland 时跳过；现在同时处理单元测试环境中 platformName 为空的情况 3. 标注了后续迁移至 login1 接口的意图 Log: 扩展屏幕锁定初始化条件，兼容单元测试和 Wayland 环境
Contributor
Author
|
/forcemerge |
|
This pr force merged! (status: blocked) |
deepin pr auto review你好！我是CodeGeeX，你的智能编程助手。我已仔细审查了你提供的Git Diff代码。下面我将从语法逻辑、代码质量、代码性能和代码安全四个方面为你提供详细的审查意见和改进建议。 总体评价这次修改的主要目的是在非Wayland平台且平台名称不为空的情况下初始化屏幕锁定状态，以避免在单元测试等没有图形平台的环境下引发错误。修改方向是正确的，但实现细节上存在一些可以优化的空间。 一、 语法逻辑
二、 代码质量
三、 代码性能
四、 代码安全
改进后的代码建议结合以上分析，我为你提供一份优化后的代码参考： diff --git a/panels/notification/server/notificationmanager.cpp b/panels/notification/server/notificationmanager.cpp
index a02efa10a..c1d2e3f4g 100644
--- a/panels/notification/server/notificationmanager.cpp
+++ b/panels/notification/server/notificationmanager.cpp
@@ -81,7 +81,11 @@ NotificationManager::NotificationManager(QObject *parent)
if(!config->value("notificationCleanupDays").isNull()) {
m_cleanupDays = config->value("notificationCleanupDays").toInt();
}
- if (QStringLiteral("wayland") != QGuiApplication::platformName()) {
+ // TODO: Subsequent migration to the login1 interface
+ const QString platformName = QGuiApplication::platformName();
+ if (!platformName.isEmpty()
+ && platformName != QStringLiteral("wayland")) {
initScreenLockedState();
}
}更严格的安全版本（如果存在 QGuiApplication 未初始化的可能）如果该类有可能在 // TODO: Subsequent migration to the login1 interface
if (qApp) {
const QString platformName = QGuiApplication::platformName();
if (!platformName.isEmpty()
&& platformName != QStringLiteral("wayland")) {
initScreenLockedState();
}
}修改点总结：
|
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
3 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.
Log: Refactor Wayland session check into initScreenLockedState using Dtk API
refactor(notification): 将 Wayland 检测移入 initScreenLockedState
Log: 用 Dtk API 重构 Wayland 检测逻辑，移入 initScreenLockedState
Summary by Sourcery
Refactor notification screen lock initialization to centralize and modernize Wayland platform detection.
Enhancements: