Skip to content

fix(shot): remember save location only after a successful save - #952

Merged
deepin-bot[bot] merged 2 commits into
linuxdeepin:masterfrom
dengzhongyuan365-dev:bug-fix-1012
Oct 12, 2026
Merged

deepin-bot[bot] merged 2 commits into
linuxdeepin:masterfrom
dengzhongyuan365-dev:bug-fix-1012

Conversation

@dengzhongyuan365-dev

@dengzhongyuan365-dev dengzhongyuan365-dev commented Oct 12, 2026 •

Copy link
Copy Markdown
Member

Stop clearing save_dir and flipping save_dir_change before the image is actually written; only persist the chosen folder after saveImg succeeds.

保存失败不再清空 save_dir、也不再把「保存时更新位置」提前改成固定路径,
避免下次启动静默存到桌面;成功后才记住所选位置。同时修正菜单勾选落到
隐藏项、导致可见菜单项没有选中状态的问题。

Log: 修复保存失败后静默存到桌面与保存位置未选中
PMS: BUG-378117
Influence: 保存失败时保留原配置,下次仍然弹出位置选择框;保存成功后菜单
正确选中已保存的位置。

Summary by Sourcery

Persist screenshot save-location changes only after successful saves and align shortcut presentation and save-menu selection across the interface.

New Features:

  • Expose save, clipboard, and tool keyboard shortcuts consistently in menus and tooltips.

Bug Fixes:

  • Preserve the configured save location and save-location mode when saving fails, and only persist a newly selected folder after a successful save.
  • Ensure invalid fixed save paths fail without mutating configuration and that the visible save-location menu item receives the checked state.

Enhancements:

  • Improve shortcut listings with clearer tool grouping, default fallbacks, and standardized shortcut formatting.

Tests:

  • Add coverage for preserving save configuration on save failures and maintaining correct visible save-menu selection.

Chores:

  • Update Chinese translations for revised shortcut labels and newly exposed UI text.

Stop clearing save_dir and flipping save_dir_change before the image is
actually written; only persist the chosen folder after saveImg succeeds.

保存失败不再清空 save_dir、也不再把「保存时更新位置」提前改成固定路径,
避免下次启动静默存到桌面;成功后才记住所选位置。同时修正菜单勾选落到
隐藏项、导致可见菜单项没有选中状态的问题。

Log: 修复保存失败后静默存到桌面与保存位置未选中
PMS: BUG-378117
Influence: 保存失败时保留原配置,下次仍然弹出位置选择框;保存成功后菜单
正确选中已保存的位置。
@sourcery-ai

sourcery-ai Bot commented Oct 12, 2026

Copy link
Copy Markdown

Sorry @dengzhongyuan365-dev, you've used your own review budget of 250,000 diff characters for the last 7 days.

You can request another review in 2 days and 18 hours by commenting @sourcery-ai review. Upgrade to get a review now.

@sourcery-ai

sourcery-ai Bot commented Oct 12, 2026

Copy link
Copy Markdown

Reviewer's Guide

The PR makes save-location configuration updates transactional: failed saves preserve the existing configuration and continue prompting for a location, while successful saves persist the newly chosen directory. It also corrects save-menu checkmarks so the selected state remains on a visible option, with regression tests covering both behaviors.

Sequence diagram for transactional screenshot save location

sequenceDiagram
    participant User
    participant MainWindow
    participant QFileDialog
    participant ImageWriter
    participant ConfigSettings

    User->>MainWindow: saveAction(pix)
    MainWindow->>ConfigSettings: getValue(save_dir)
    MainWindow->>ConfigSettings: getValue(save_dir_change)
    alt invalid path and choose-on-save enabled
        MainWindow->>QFileDialog: getExistingDirectory()
        QFileDialog-->>MainWindow: selected directory
    else invalid fixed path
        MainWindow-->>User: saveAction returns false
    end
    MainWindow->>ImageWriter: saveImg(...)
    alt save succeeds
        ImageWriter-->>MainWindow: success
        MainWindow->>ConfigSettings: setValue(save_dir, selected directory)
        MainWindow->>ConfigSettings: setValue(save_dir_change, false)
        MainWindow-->>User: save succeeds
    else save fails
        ImageWriter-->>MainWindow: failure
        MainWindow-->>User: saveAction returns false
        Note over ConfigSettings: Existing save location configuration is preserved
    end
Loading

Flow diagram for save-menu location checkmarks

flowchart TD
    A[Load save-location configuration] --> B{Specified location with choose-on-save?}
    B -- No --> C[Keep visible menu selection]
    B -- Yes --> D{Update-on-save enabled and custom path exists?}
    D -- Yes --> E[Check update-on-save]
    D -- No --> F[Leave update-on-save unchecked]
    F --> G[Visible choose-on-save option retains correct state]
Loading

File-Level Changes

Change Details Files
Defer persistence of the selected save directory until the image write completes successfully.
  • Stop clearing the configured save directory during save notifications.
  • Track a newly selected directory in memory and commit it only after successful save processing.
  • Preserve configuration and return failure when a fixed save directory is invalid instead of silently falling back to the desktop.
  • Move the save-directory update flag reset to the successful-save path.
src/main_window.cpp
Ensure save-location menu state reflects only visible and applicable choices.
  • Normalize invalid configured paths to choose-on-save mode and persist the fallback state.
  • Only check the update-on-save action when a historical custom path exists, preventing hidden actions from appearing selected.
src/widgets/savemenumanager.cpp
Add regression coverage for configuration preservation, invalid paths, and visible menu selection.
  • Verify save notifications no longer clear the configured directory.
  • Verify invalid fixed paths fail without mutating configuration.
  • Verify exactly one visible location action is checked with and without a saved path.
  • Verify invalid configured paths fall back to choose-on-save state.
tests/ut_screen_shot_recorder/ut_main_window_cov2.h
tests/ut_screen_shot_recorder/widgets/ut_savemenumanager_cov.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

@deepin-ci-robot

Copy link
Copy Markdown

deepin pr auto review

🤖 AI 代码审查报告

总体评分: 99 分 (通过阈值: 70分)

Pass


📊 总体评价

项目 结果
审查结论 代码审查通过
评分详情 代码质量优秀,修复了保存失败后配置被错误清空的Bug,逻辑改进合理,测试覆盖充分,未发现安全问题。

🔍 详细分析

1. 语法逻辑 ✅

评价: 优秀 ✅ 通过

潜在问题:
✅ 未发现明显问题

建议: 继续保持良好的逻辑结构和边界条件处理


2. 代码质量 ✅

评价: 优秀 ✅ 通过

潜在问题:

  1. src/widgets/savemenumanager.cpp:281 - initializeFromConfig() 函数在初始化过程中产生副作用(写入配置),理想情况下初始化函数不应有写操作。此处是有意为之,用于持久化回退状态,且有日志记录。

建议: 可考虑将配置持久化操作分离到单独的函数中,在初始化完成后显式调用


3. 代码性能 ✅

评价: 优秀 ✅ 通过

潜在问题:
✅ 未发现明显问题

建议: 继续保持良好的性能实践


4. 代码安全 🔒

评价: 优秀 ✅ 通过

🔐 发现 0 个安全漏洞
漏洞对比统计:新增漏洞 0 个,减少漏洞 0 个,持平 0 个

安全漏洞详情:
✅ 未发现安全漏洞

建议: 继续保持安全编码实践


💡 改进建议代码示例

// 建议将 initializeFromConfig() 中的配置持久化分离到单独函数
void SaveMenuManager::persistFallbackState()
{
    ConfigSettings::instance()->setValue("shot", "location_state",
        static_cast<int>(LocationState::ChooseOnSave));
    ConfigSettings::instance()->setValue("shot", "save_dir_change", true);
}

// 在 initializeFromConfig() 完成后调用
void SaveMenuManager::initializeFromConfig()
{
    // ... 初始化逻辑 ...
    if (invalidPath) {
        m_currentLocationState = LocationState::ChooseOnSave;
        updateSubMenuForFirstTime();
        m_chooseOnSaveAction->setChecked(true);
        persistFallbackState(); // 显式调用持久化
    }
}

本报告由 AI 代码审查工具自动生成

lzwind
lzwind previously approved these changes Oct 12, 2026
Add shortcut hints to toolbar tooltips and align the shortcut panel.
为工具栏提示补充快捷键,并同步调整快捷键面板内容与顺序。

Log: 完善工具栏提示和快捷键面板
PMS: TASK-393987
Influence: 工具栏提示和快捷键面板显示更完整,Ctrl+S 行为改为保存。
@deepin-ci-robot

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: dengzhongyuan365-dev

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

1 similar comment
@deepin-ci-robot

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: dengzhongyuan365-dev

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

@dengzhongyuan365-dev

Copy link
Copy Markdown
Member Author

/forcemerge

@deepin-bot

deepin-bot Bot commented Oct 12, 2026

Copy link
Copy Markdown
Contributor

This pr force merged! (status: blocked)

@deepin-bot
deepin-bot Bot merged commit 8466493 into linuxdeepin:master Oct 12, 2026
5 of 7 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