Skip to content

Conversation

@BLumia
Copy link
Member

@BLumia BLumia commented Jun 5, 2025

使用 ICU 提供相对时间格式的时间展示,避免拼接字符串造成不易于本地化的问题.

Summary by Sourcery

Use ICU libraries to provide localized relative datetime formatting in notification items, replacing manual string concatenation for improved i18n support.

Enhancements:

  • Replace manual minute/hour/day string concatenation with ICU RelativeDateTimeFormatter for localized relative times
  • Introduce SimpleDateFormat and formatter.combineDateAndTime to handle 'yesterday' time display via ICU
  • Add utility functions to convert between ICU UnicodeString and QString
  • Use QLocale::system().dateFormat for older date formatting

Build:

  • Add ICU uc, i18n, and io dependencies in CMakeLists and link ICU libraries

@BLumia BLumia requested a review from 18202781743 June 5, 2025 12:03
@sourcery-ai
Copy link

sourcery-ai bot commented Jun 5, 2025

Reviewer's Guide

This PR replaces manual string concatenation for relative date/time displays with ICU’s locale-aware RelativeDateTimeFormatter and SimpleDateFormat, adds utility converters between QString and icu::UnicodeString, and updates CMake and packaging to include ICU dependencies.

Sequence Diagram: Relative Time Formatting with ICU in AppNotifyItem

sequenceDiagram
    title "Sequence Diagram: Relative Time Formatting with ICU in AppNotifyItem"
    participant A as "AppNotifyItem"
    participant RDF as "icu::RelativeDateTimeFormatter"
    participant SDF as "icu::SimpleDateFormat"
    participant Conv as "StringConversionUtils"

    A ->> A: updateTime()
    A ->> RDF: create(Locale::getDefault(), ..., UDAT_STYLE_LONG, ...)
    alt If time is within minutes/hours (same day)
        A ->> RDF: format(value, UDAT_DIRECTION_LAST, UDAT_RELATIVE_MINUTES/HOURS, result, status)
        A ->> Conv: toQString(result)
        Conv -->> A: formattedTime : QString
    else If time is "Yesterday"
        A ->> RDF: format(1, UDAT_DIRECTION_LAST, UDAT_RELATIVE_DAYS, dateResult, status)
        A ->> SDF: create("HH:mm", Locale::getDefault(), status)
        A ->> SDF: format(m_entity.cTime(), timeString, status)
        A ->> RDF: combineDateAndTime(dateResult, timeString, combinedResult, status)
        A ->> Conv: toQString(combinedResult)
        Conv -->> A: formattedTime : QString
    end
Loading

File-Level Changes

Change Details Files
Use ICU for formatting relative dates and times
  • Add ICU headers for RelativeDateTimeFormatter and SimpleDateFormat
  • Implement toQString/fromQString converters for UnicodeString interoperability
  • Refactor AppNotifyItem::updateTime to call formatter.format and combineDateAndTime
  • Replace Qt-based tr() calls with ICU formatting results
panels/notification/center/notifyitem.cpp
Include ICU libraries in build and packaging
  • Add find_package(ICU 74.2) with uc, i18n, io components
  • Link ICU::uc, ICU::i18n, ICU::io in target_link_libraries
  • Update debian/control to require ICU development packages
panels/notification/CMakeLists.txt
CMakeLists.txt
debian/control

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

Copy link

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

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

Hey @BLumia - I've reviewed your changes - here's some feedback:

  • Consider moving the ICU RelativeDateTimeFormatter and SimpleDateFormat instances out of updateTime (e.g. as static or member variables) to avoid recreating them on every call and improve performance.
  • After each ICU operation you should check UErrorCode (status and timeStatus) and handle or log errors to avoid silent formatting failures.
Here's what I looked at during the review
  • 🟡 General issues: 4 issues found
  • 🟢 Security: all looks good
  • 🟢 Testing: all looks good
  • 🟢 Complexity: all looks good
  • 🟢 Documentation: all looks good

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@BLumia BLumia force-pushed the icu-relative-dateformat branch 2 times, most recently from 43dc5d8 to 56fd120 Compare June 5, 2025 12:13
使用 ICU 提供相对时间格式的时间展示,避免拼接字符串造成不易于本地化
的问题.

Log:
@BLumia BLumia force-pushed the icu-relative-dateformat branch from 56fd120 to 0d0b3d9 Compare June 5, 2025 12:17
@deepin-ci-robot
Copy link

deepin pr auto review

关键摘要:

  • CMakeLists.txt中添加了对ICU库的依赖,确保在构建时正确链接ICU库。
  • debian/control文件中添加了libicu-dev作为构建依赖,确保在Debian包构建时能够找到ICU库。
  • panels/notification/CMakeLists.txt中添加了对ICU库的链接,确保编译时能够使用ICU的功能。
  • notifyitem.cpp文件中添加了ICU相关的头文件,并实现了toQStringfromQString函数,用于在QString和ICU的UnicodeString之间进行转换。
  • notifyitem.cpp文件中使用了ICU的RelativeDateTimeFormatterSimpleDateFormat类来格式化日期和时间,提高了代码的可读性和可维护性。

是否建议立即修改:

  • 否,提交的代码没有明显的语法或逻辑错误。
  • 是,建议检查ICU库的版本是否与项目中使用的版本兼容,并确保在所有相关平台和环境中都能正确链接和使用ICU库。
  • 是,建议添加对ICU库的依赖和链接的文档说明,以便其他开发者理解这些更改的目的和影响。
  • 是,建议对toQStringfromQString函数进行单元测试,确保它们在不同环境下都能正确工作。

@deepin-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: 18202781743, BLumia

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

Details 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

@BLumia BLumia merged commit c40886d into linuxdeepin:master Jun 9, 2025
7 of 10 checks passed
@BLumia BLumia deleted the icu-relative-dateformat branch June 9, 2025 05:09
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