Skip to content

fix: failed to save too long filename for DPrintPreviewWidget#644

Merged
18202781743 merged 1 commit intolinuxdeepin:masterfrom
18202781743:master
Mar 6, 2025
Merged

fix: failed to save too long filename for DPrintPreviewWidget#644
18202781743 merged 1 commit intolinuxdeepin:masterfrom
18202781743:master

Conversation

@18202781743
Copy link
Contributor

Truncate file name to limit file's length.

pms: BUG-297841

Truncate file name to limit file's length.

pms: BUG-297841
@18202781743 18202781743 requested review from BLumia and mhduiy March 6, 2025 07:24
@deepin-ci-robot
Copy link
Contributor

deepin pr auto review

关键摘要:

  • truncateFileName 函数中的 maxLength 参数默认值设置为255,但通常文件路径的最大长度限制可能低于255字符。
  • truncateFileName 函数在处理文件名截断时,没有考虑文件扩展名,可能会导致截断后的文件名无效。
  • saveImageToFile 函数中使用了 QtConcurrent::run 进行多线程保存文件,但没有处理线程池的线程数量限制,可能会导致线程过多。
  • saveImageToFile 函数中使用了 qWarning 输出错误信息,但没有提供错误处理机制,例如重试或通知用户。

是否建议立即修改:

  • 应该确认 maxLength 参数的默认值是否适合文件路径的最大长度限制。
  • 需要改进 truncateFileName 函数,确保截断后的文件名仍然有效,包括文件扩展名。
  • 应该限制 QThreadPool 的线程数量,以避免过多的线程创建。
  • 应该提供更完善的错误处理机制,例如重试保存操作或通知用户保存失败。

deepin-ci-robot added a commit to linuxdeepin/dtk6widget that referenced this pull request Mar 6, 2025
Synchronize source files from linuxdeepin/dtkwidget.

Source-pull-request: linuxdeepin/dtkwidget#644
@deepin-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

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

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

@18202781743 18202781743 merged commit 65b8848 into linuxdeepin:master Mar 6, 2025
19 of 21 checks passed
18202781743 pushed a commit to linuxdeepin/dtk6widget that referenced this pull request Mar 6, 2025
Synchronize source files from linuxdeepin/dtkwidget.

Source-pull-request: linuxdeepin/dtkwidget#644
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