Skip to content

fix: a display logic error in the secondary prompt.#458

Merged
deepin-bot[bot] merged 1 commit intolinuxdeepin:release/eaglefrom
lichaofan2008:release/eagle
Mar 10, 2026
Merged

fix: a display logic error in the secondary prompt.#458
deepin-bot[bot] merged 1 commit intolinuxdeepin:release/eaglefrom
lichaofan2008:release/eagle

Conversation

@lichaofan2008
Copy link
Contributor

@lichaofan2008 lichaofan2008 commented Mar 10, 2026

There is a display logic error in the secondary prompt, and it should be consistent with the display logic of the primary prompt.
二级提示的显示逻辑错误,应该和一级提示的显示逻辑保持一致。

Summary by Sourcery

Bug Fixes:

  • Fix incorrect display logic where the secondary camera error prompt was shown instead of hidden during video preview countdown.

There is a display logic error in the secondary prompt, and it should be consistent with the display logic of the primary prompt.
二级提示的显示逻辑错误,应该和一级提示的显示逻辑保持一致。
@sourcery-ai
Copy link

sourcery-ai bot commented Mar 10, 2026

Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

Aligns the secondary camera error prompt visibility logic with the primary prompt when entering VIDEO preview state to fix an incorrect display condition.

Updated class diagram for videowidget countdown and camera error prompts

classDiagram
  class videowidget {
    +showCountDownLabel(state PREVIEW_ENUM_STATE) void
    -m_pCamErrItem CamErrItem
    -m_pCamErrItemSub CamErrItem
    -m_pSvgItem SvgItem
  }

  class PREVIEW_ENUM_STATE {
    <<enumeration>>
    PREVIEW_ENUM_STATE_IDLE
    PREVIEW_ENUM_STATE_VIDEO
    PREVIEW_ENUM_STATE_OTHER
  }

  videowidget --> PREVIEW_ENUM_STATE : uses
  videowidget --> CamErrItem : manages
  videowidget --> SvgItem : manages
Loading

State diagram for VIDEO preview prompt visibility logic

stateDiagram-v2
  [*] --> PREVIEW_ENUM_STATE_IDLE

  PREVIEW_ENUM_STATE_IDLE --> PREVIEW_ENUM_STATE_VIDEO : showCountDownLabel(PREVIEW_ENUM_STATE_VIDEO)

  state PREVIEW_ENUM_STATE_VIDEO {
    [*] --> VIDEO_active
    VIDEO_active --> VIDEO_active : updatePromptVisibility
  }

  VIDEO_active : entry / hide m_pCamErrItem
  VIDEO_active : entry / hide m_pCamErrItemSub
  VIDEO_active : entry / hide m_pSvgItem
Loading

File-Level Changes

Change Details Files
Correct secondary camera error prompt visibility when entering VIDEO preview state.
  • In the VIDEO preview branch of showCountDownLabel, change the secondary camera error item from being shown to being hidden so it matches the intended display logic of the primary prompt.
  • Keep primary camera error item hidden and SVG item hidden as before, preserving existing behavior aside from the secondary prompt toggle.
src/src/videowidget.cpp

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

@deepin-ci-robot
Copy link

deepin pr auto review

这段代码修改主要是针对 VIDEO 状态下 UI 元素显示逻辑的调整。以下是对该 git diff 的详细审查意见:

1. 代码逻辑审查

  • 修改内容:将 m_pCamErrItemSub->show() 修改为 m_pCamErrItemSub->hide()
  • 分析
    • case VIDEO 分支中,逻辑表示当前处于视频预览或播放状态。
    • m_pCamErrItem 被隐藏(hide()),这通常意味着主错误提示被关闭。
    • 原代码中,m_pCamErrItemSub(可能是子错误提示或辅助信息)被显示(show())。但在视频正常播放的状态下,显示“错误提示”通常是不合理的,这会导致 UI 上出现遮挡或误导信息。
    • 结论:修改为 hide() 是合理的,确保在进入视频播放状态时,清除所有与错误或状态异常相关的 UI 元素,保持界面整洁。

2. 代码质量与规范

  • 命名规范:变量命名如 m_pCamErrItemSub 符合 Qt/C++ 的常见命名习惯(m_ 前缀表示成员变量,p 表示指针),可读性尚可。
  • 代码风格:代码缩进和格式保持一致,符合 Qt 的代码风格。

3. 代码性能

  • 影响:此修改仅涉及 UI 组件的显示/隐藏函数调用,不涉及复杂的计算或循环,对性能无显著影响。

4. 代码安全

  • 空指针检查
    • 代码中直接调用了 m_pCamErrItemSub->hide()。如果 m_pCamErrItemSub 指针未被初始化或已被置为 nullptr,直接调用会导致程序崩溃(Segmentation Fault)。
    • 建议:虽然 Qt 的 QObject 及其子类在析构时通常会自动将指向它的子对象指针置空(如果使用了 QPointerQObject::deleteLater 等机制),但在直接操作成员指针前,增加判空逻辑是一个良好的防御性编程习惯。
    • 改进代码示例
      if (m_pCamErrItemSub) {
          m_pCamErrItemSub->hide();
      }

5. 综合改进建议

建议对整个 case VIDEO 分支中的指针调用增加安全性检查,以防止潜在的空指针解引用风险。

改进后的代码片段建议:

case VIDEO:
    if (m_pCamErrItem) {
        m_pCamErrItem->hide();
    }
    if (m_pCamErrItemSub) {
        m_pCamErrItemSub->hide(); // 修改点:确保在视频状态下隐藏子错误项
    }
    if (m_pSvgItem) {
        m_pSvgItem->hide();
    }

    if (!get_capture_pause()) { ... }

总结
该修改在逻辑上是正确的,修复了视频状态下错误提示未隐藏的 Bug。为了提高代码的健壮性,建议补充空指针检查。

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 - I've left some high level feedback:

  • Since the secondary prompt is now meant to mirror the primary prompt’s behavior, consider extracting their shared display logic into a helper method so they can’t silently drift apart again.
  • It may be worth clarifying the naming of m_pCamErrItem and m_pCamErrItemSub or adding a brief inline comment near this branch to make it immediately obvious why the secondary error item is hidden in the VIDEO state while the primary is shown.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- Since the secondary prompt is now meant to mirror the primary prompt’s behavior, consider extracting their shared display logic into a helper method so they can’t silently drift apart again.
- It may be worth clarifying the naming of `m_pCamErrItem` and `m_pCamErrItemSub` or adding a brief inline comment near this branch to make it immediately obvious why the secondary error item is hidden in the `VIDEO` state while the primary is shown.

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.

@deepin-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: lichaofan2008, max-lvs

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

@lichaofan2008
Copy link
Contributor Author

/merge

@deepin-bot deepin-bot bot merged commit 7420a34 into linuxdeepin:release/eagle Mar 10, 2026
19 checks passed
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