Skip to content

sync: from linuxdeepin/dde-session-shell #420

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

deepin-ci-robot
Copy link
Contributor

@deepin-ci-robot deepin-ci-robot commented May 20, 2025

Synchronize source files from linuxdeepin/dde-session-shell.

Source-pull-request: linuxdeepin/dde-session-shell#8

Summary by Sourcery

Enable optional region-format synchronization for date and time display via D-Bus when compiled with ENABLE_DSS_SNIPE.

New Features:

  • Add D-Bus integration in CenterTopWidget to acquire and listen for user-specific locale, long date, and short time format settings.
  • Implement updateUserDateTimeFormat logic to fetch region-format values and refresh the TimeWidget dynamically.
  • Extend TimeWidget to accept custom short time and long date formats and apply them when provided.

@deepin-ci-robot
Copy link
Contributor Author

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: deepin-ci-robot

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Synchronize source files from linuxdeepin/dde-session-shell.

Source-pull-request: linuxdeepin/dde-session-shell#8
@deepin-ci-robot
Copy link
Contributor Author

deepin pr auto review

代码审查意见:

  1. 代码重复

    • getRegionFormatConfigPathgetRegionFormatValue 函数中,错误处理逻辑是相同的,可以考虑将错误处理逻辑提取到一个单独的函数中,以减少代码重复。
  2. 资源管理

    • updateRegionFormatConnection 函数中,当 m_currentUser 不为空时,应该先断开连接,然后再连接新的用户。这样可以避免潜在的连接泄漏。
  3. 空指针检查

    • getRegionFormatConfigPathgetRegionFormatValue 函数中,对 useruserConfigDbusPath 的空指针检查是必要的,但应该确保这些函数的调用者也进行类似的检查。
  4. 宏定义的使用

    • 使用 #ifdef ENABLE_DSS_SNIPE 宏定义来控制代码的编译,这是一个好的做法,但应该确保这个宏在所有相关文件中都被正确定义。
  5. 代码风格

    • 代码缩进和格式应该保持一致,例如在 updateUserDateTimeFormat 函数中,if 语句的缩进不一致。
  6. 注释

    • updateUserDateTimeFormat 函数中,对 m_shortTimeFormatm_longDateFormat 的空字符串检查应该添加注释说明为什么需要这个检查。
  7. 性能考虑

    • updateUserDateTimeFormat 函数中,每次更新用户日期时间格式时都会调用 refreshTime 函数,如果这个函数中有一些昂贵的操作,可能会影响性能。可以考虑优化 refreshTime 函数,或者只在必要时调用它。
  8. 安全性

    • 在与 D-Bus 交互的函数中,应该对传入的参数进行验证,以防止潜在的注入攻击。

综合以上意见,代码在逻辑上和功能上看起来是正确的,但存在一些可以改进的地方,以提高代码的可维护性、性能和安全性。

Copy link

sourcery-ai bot commented May 20, 2025

Reviewer's Guide

This PR conditionally adds D-Bus–driven region format synchronization under the ENABLE_DSS_SNIPE guard: it hooks into the per-user config manager to watch locale/time/date settings, introduces helpers to fetch those settings, and extends the time widget to render using custom formats.

Sequence Diagram for Region Format Setup on User Switch (ENABLE_DSS_SNIPE)

sequenceDiagram
    title Sequence Diagram for Region Format Setup on User Switch (ENABLE_DSS_SNIPE)
    participant CTW as CenterTopWidget
    participant QDBus as "QDBusConnection.systemBus()"
    participant RootConfigMgr as "org.desktopspec.ConfigManager (on '/')"
    participant UserConfigMgr as "org.desktopspec.ConfigManager.Manager (on user path)"
    participant TW as TimeWidget

    Note over CTW: Inside setCurrentUser(newUser)
    CTW->>CTW: updateRegionFormatConnection(newUser)
    activate CTW # updateRegionFormatConnection
    opt m_currentUser exists
        CTW->>RootConfigMgr: getRegionFormatConfigPath(m_currentUser) (calls acquireManagerV2)
        activate RootConfigMgr
        RootConfigMgr-->>CTW: oldDbusPath
        deactivate RootConfigMgr
        CTW->>QDBus: disconnect(oldDbusPath, "org.desktopspec.ConfigManager.Manager", "valueChanged", ...)
    end
    CTW->>RootConfigMgr: getRegionFormatConfigPath(newUser) (calls acquireManagerV2)
    activate RootConfigMgr
    RootConfigMgr-->>CTW: newDbusPath
    deactivate RootConfigMgr
    CTW->>QDBus: connect(newDbusPath, "org.desktopspec.ConfigManager.Manager", "valueChanged", ...)
    deactivate CTW # updateRegionFormatConnection

    CTW->>CTW: updateUserDateTimeFormat()
    activate CTW # updateUserDateTimeFormat
    CTW->>RootConfigMgr: getRegionFormatConfigPath(m_currentUser) (calls acquireManagerV2)
    activate RootConfigMgr
    RootConfigMgr-->>CTW: userConfigDbusPath
    deactivate RootConfigMgr
    Note over CTW: UserConfigMgr represents interface on userConfigDbusPath
    CTW->>UserConfigMgr: getRegionFormatValue(localeNameKey) (calls value)
    activate UserConfigMgr
    UserConfigMgr-->>CTW: localeName
    deactivate UserConfigMgr
    CTW->>UserConfigMgr: getRegionFormatValue(shortTimeFormatKey) (calls value)
    activate UserConfigMgr
    UserConfigMgr-->>CTW: shortTimeFormat
    deactivate UserConfigMgr
    CTW->>UserConfigMgr: getRegionFormatValue(longDateFormatKey) (calls value)
    activate UserConfigMgr
    UserConfigMgr-->>CTW: longDateFormat
    deactivate UserConfigMgr
    CTW->>TW: updateLocale(localeName, shortTimeFormat, longDateFormat)
    activate TW
    TW->>TW: refreshTime()
    deactivate TW
    deactivate CTW # updateUserDateTimeFormat
Loading

File-Level Changes

Change Details Files
Hook into user-specific region-format change events via D-Bus in CenterTopWidget
  • Added onUserRegionFormatValueChanged slot to handle valueChanged signals
  • Called updateRegionFormatConnection in setCurrentUser to manage signal connections
  • Connected/disconnected systemBus ‘valueChanged’ signals for org.desktopspec.ConfigManager.Manager
  • Triggered updateUserDateTimeFormat when relevant keys change
src/widgets/centertopwidget.cpp
src/widgets/centertopwidget.h
Introduce helpers to fetch and apply region-format settings
  • Implemented getRegionFormatConfigPath to acquire user config path via D-Bus
  • Implemented getRegionFormatValue to query specific format keys
  • Added updateUserDateTimeFormat to read localeName, shortTimeFormat, longDateFormat and update TimeWidget
src/widgets/centertopwidget.cpp
src/widgets/centertopwidget.h
Extend TimeWidget to support custom time and date formats
  • Added m_shortTimeFormat and m_longDateFormat members
  • Added updateLocale overload accepting locale and custom format strings
  • Adjusted refreshTime to render using custom shortTimeFormat and longDateFormat when set
src/widgets/timewidget.cpp
src/widgets/timewidget.h
Guard all new region-format and locale code behind ENABLE_DSS_SNIPE macro
  • Wrapped static key definitions, slots, methods, and calls with #ifdef ENABLE_DSS_SNIPE
  • Ensured no behavior changes when the macro is undefined
  • Aligned header and source files to match conditional compilation
src/widgets/centertopwidget.cpp
src/widgets/centertopwidget.h
src/widgets/timewidget.cpp
src/widgets/timewidget.h

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

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.

1 participant