sync: from linuxdeepin/dde-session-shell#505
Conversation
Synchronize source files from linuxdeepin/dde-session-shell. Source-pull-request: linuxdeepin/dde-session-shell#67
Reviewer's GuideSynchronizes the greeter-display-setting utility and related build/install logic with upstream, adjusting how it is built, linked, installed, and how it computes Qt scale factors depending on whether the DSS SNIPE variant is enabled. Flow diagram for Qt scale factor selection in greeter-display-settingflowchart TD
main[main] --> cond_env{ENABLE_DSS_SNIPE}
cond_env -->|true and !IsWayland| call_set1[setQtScaleFactorEnv]
cond_env -->|true and IsWayland| end_main1[exit]
cond_env -->|false and IsWayland| call_set2[setQtScaleFactorEnv]
cond_env -->|false and !IsWayland| end_main2[exit]
subgraph setQtScaleFactorEnv
setQtScaleFactorEnv --> cond_scale{ENABLE_DSS_SNIPE}
cond_scale -->|true| sf1[getScaleFactor]
cond_scale -->|false and IsWayland| sf2[getScaleFormConfig]
cond_scale -->|false and !IsWayland| sf3[getScaleFactor]
sf1 --> log[print QT_SCALE_FACTOR if > 0]
sf2 --> log
sf3 --> log
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
deepin pr auto review你好！我是CodeGeeX，你的智能编程助手。我已仔细审查了你提供的 Git Diff 输入。本次修改主要涉及 CMake 构建逻辑调整、Shell 脚本调用变更以及 C++ 代码的条件编译逻辑修改。 以下是我从语法逻辑、代码质量、代码性能和代码安全四个维度提出的审查意见和改进建议： 1. 语法与逻辑
2. 代码质量
3. 代码性能
4. 代码安全
综合修改建议代码C++ ( // SPDX-FileCopyrightText: 2015 - 2024 UnionTech Software Technology Co., Ltd.
//
// SPDX-License-Identifier: GPL-3.0-or-later
// ... 前置代码 ...
static void setQtScaleFactorEnv() {
double scaleFactor = 0.0;
// 建议：如果逻辑不是刻意反转，这里应该统一为 IsWayland
#ifdef ENABLE_DSS_SNIPE
scaleFactor = getScaleFactor(); // 请确认：Snipe模式下 X11 真的需要设置吗？
#else
scaleFactor = IsWayland ? getScaleFormConfig() : getScaleFactor();
#endif
qDebug() << "Final scale factor: " << scaleFactor;
// 增加安全边界校验，防止异常值输出
if (scaleFactor > 0.0 && scaleFactor <= 10.0) {
std::cout << QString("QT_SCALE_FACTOR=%1").arg(scaleFactor).toStdString() << std::endl;
}
}
int main(int argc, char* argv[])
{
// 建议：如果 Snipe 模式下也是 Wayland 需要设置，请将 #ifdef 逻辑统一
#ifdef ENABLE_DSS_SNIPE
// 此处逻辑与注释相悖，如果是刻意为之，请加详尽注释；否则应改为 if (IsWayland)
if (!IsWayland) {
setQtScaleFactorEnv();
}
#else
if (IsWayland) {
setQtScaleFactorEnv();
}
#endif
return 0;
}Shell ( xsettingsd_conf="/etc/lightdm/deepin/xsettingsd.conf"
if [ -e "$xsettingsd_conf" ]; then
xsettingsd -c "$xsettingsd_conf" &
else
# 直接执行并捕获输出，避免 TOCTOU 漏洞和多余的 tail 进程
if scale_env=$(/usr/bin/greeter-display-setting 2>/dev/null); then
# 务必加上双引号，防止词法分割
export "$scale_env"
fi
fiCMake ( # 在 snipe 分支或全局配置中确保宏生效
if (NOT DISABLE_DSS_SNIPE)
target_compile_definitions(greeter-display-setting PRIVATE ENABLE_DSS_SNIPE)
endif() |
![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:
- The
ENABLE_DSS_SNIPE/IsWaylandbranching for scale-factor handling is split betweensetQtScaleFactorEnv()andmain(), which makes the control flow harder to reason about; consider centralizing the condition in one place (e.g., compute the desired scaleFactor inmain()and pass it in) so the behavior for each build configuration is clearer and less error-prone to change.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `ENABLE_DSS_SNIPE`/`IsWayland` branching for scale-factor handling is split between `setQtScaleFactorEnv()` and `main()`, which makes the control flow harder to reason about; consider centralizing the condition in one place (e.g., compute the desired scaleFactor in `main()` and pass it in) so the behavior for each build configuration is clearer and less error-prone to change.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: deepin-ci-robot, yixinshark 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 |
2 participants
Synchronize source files from linuxdeepin/dde-session-shell.
Source-pull-request: linuxdeepin/dde-session-shell#67
Summary by Sourcery
Update greeter-display-setting build and scaling behavior and unify greeter installation targets.
Enhancements: