Skip to content

fix: update save state#206

Merged
lzwind merged 1 commit into
linuxdeepin:masterfrom
JWWTSL:master
Jun 4, 2026
Merged

fix: update save state#206
lzwind merged 1 commit into
linuxdeepin:masterfrom
JWWTSL:master

Conversation

@JWWTSL

@JWWTSL JWWTSL commented Jun 3, 2026

Copy link
Copy Markdown
Contributor

log: Remove premature _dirty=0 assignments before save calls so that setDirty(false) can properly emit dirtyChanged after a successful save, allowing the tab title * prefix to be cleared.

pms: bug-364361

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Sorry @JWWTSL, you have reached your weekly rate limit of 500000 diff characters.

Please try again later or upgrade to continue using Sourcery

@JWWTSL JWWTSL force-pushed the master branch 3 times, most recently from c31c34e to abddda1 Compare June 4, 2026 05:31
log: Remove premature _dirty=0 assignments before save calls so that
setDirty(false) can properly emit dirtyChanged after a successful
save, allowing the tab title * prefix to be cleared.

pms: bug-364361
@deepin-ci-robot

Copy link
Copy Markdown

deepin pr auto review

你好!我是CodeGeeX,你的智能编程助手。我已经仔细审查了你提供的Git Diff内容。本次修改主要涉及文件保存逻辑和脏标志(dirty flag)的时序调整,以及页面标题的更新。

以下是我对本次代码变更的详细审查意见,分为语法逻辑、代码质量、代码性能和代码安全四个方面:

1. 语法与逻辑

  • 潜在的逻辑回退风险(严重)
    cdrawparamsigleton.cpp 中,你将 _dirty = 0; 从调用保存函数(saveToDdf / saveToImage之前移除了。这意味着如果保存操作失败(rsfalse),_dirty 仍然保持为真,这本身是正确的逻辑。
    但是,如果保存操作成功,代码中并没有在成功后将 _dirty 置为 0(或 false)。请确认 saveToDdfsaveToImage 内部是否在成功后主动设置了 _dirty = false。如果这两个函数内部没有处理,那么保存成功后文件依然是“未保存”状态,会导致用户反复触发保存逻辑。
    建议:如果底层保存函数不负责清理脏标志,建议在保存成功后显式清理,例如:

    rs = page()->borad()->fileHander()->saveToDdf(this, filePath);
    if (rs) {
        _dirty = false; // 确保保存成功后清除脏标志
    }
  • 页面标题设置的时机与逻辑(ccentralwidget.cpp
    你在 Page::saveelse 分支中添加了 borad()->setPageTitle(this, this->title());。从上下文推测,这可能是为了处理“另存为”或保存失败重试后的标题同步问题。
    疑问:如果 this->title() 没有发生改变,调用 setPageTitle 是否是冗余操作?如果是因为保存操作改变了文件的路径或名称,这里使用旧的 this->title() 是否能正确更新?
    建议:请确认 else 分支触发的业务场景。如果是为了同步保存后的新文件名,应该传入最新的标题(例如从文件路径提取的新名称),而不是原有的 this->title()

  • 脏标志时序调整(ccentralwidget.cpp - loadFiles
    setDirty(false) 移到 Lambda 的末尾,并且仅在 bNewPage 为真时执行,这个逻辑改进非常好。这确保了图片完全加载并添加到上下文之后,才清除脏标志,避免了加载过程中状态不一致的问题。

2. 代码质量

  • 命名规范不一致
    cdrawparamsigleton.cpp 中,代码使用了 _dirty = 0;。从变量名带有下划线前缀(_dirty)来看,它应该是一个布尔类型的成员变量(表示“脏”状态)。
    建议:在 C++ 中,布尔赋值应使用 true/false 而不是 1/0。使用 _dirty = false; 语义更清晰,符合现代 C++ 的最佳实践。

  • 拼写错误
    代码中多次出现 borad() 函数调用。
    建议:这极有可能是 board() 的拼写错误。虽然修改拼写可能涉及大量代码和接口变更,但如果是新代码或允许重构,建议修正为 board() 以提高代码可读性和可维护性。

  • Lambda 括号与格式
    loadFiles 的 Lambda 末尾新增了 if (bNewPage) currentPage()->context()->setDirty(false);
    建议:为了与大多数代码规范保持一致,并防止宏展开和调试时的行号错乱,建议即使 if 只有一行代码,也加上大括号:

    if (bNewPage) {
        currentPage()->context()->setDirty(false);
    }

3. 代码性能

  • 本次修改不涉及高频循环或复杂的算法变更,对性能没有明显负面影响。
  • 唯一需要留意的是 ccentralwidget.cpp 中的 setPageTitle,如果 save 函数在极短时间被高频触发,且 setPageTitle 内部引发了耗时的 UI 重绘或信号广播,可以考虑在调用前判断 title 是否真的发生了变化。

4. 代码安全

  • 版权年份的合法性(cdrawparamsigleton.cpp
    你将 SPDX-FileCopyrightText 的结束年份从 2022 修改为了 2026。
    建议:版权年份通常应反映代码首次发布或最近实质性修改的年份。使用未来的年份(2026)在法律和合规层面可能是不严谨的,除非有特殊的法律声明要求。建议确认项目组的版权规范,通常写当前修改年份(如2023或2024)更为合规。

  • 文件后缀判断的安全性
    代码中使用了 if ("ddf" == info.suffix().toLower())
    说明:这里使用 toLower() 进行了大小写无关的匹配,是非常好的做法,可以兼容 .DDF.Ddf 等文件,避免了大小写导致的保存逻辑错误。

总结

本次代码修改的核心逻辑(延迟脏标志的清除)是合理的,但最关键的问题在于保存成功后脏标志是否会被正确清除。请务必检查 saveToDdfsaveToImage 的内部实现,防止出现“文件已保存,但程序依然提示未保存”的 Bug。此外,注意代码规范上的小细节(如布尔赋值和拼写错误),这会大大提升代码的长期可维护性。

@deepin-ci-robot

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: JWWTSL, lzwind

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

@JWWTSL

JWWTSL commented Jun 4, 2026

Copy link
Copy Markdown
Contributor Author

/forcemerge

@lzwind lzwind merged commit a29465d into linuxdeepin:master Jun 4, 2026
16 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