fix(print): use valid file name for print preview output#178
Conversation
Reviewer's guide (collapsed on small PRs)Reviewer's GuideFixes print preview output document naming by using the currently viewed image path, filtering out empty paths, and passing only a valid file name (with a safe fallback) to the print dialog. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="libimageviewer/widgets/printhelper.cpp" line_range="72-73" />
<code_context>
}
}
- tempExsitPaths << paths;
+ if (!path.isEmpty()) {
+ tempExsitPaths << path;
+ }
}
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Align tempExsitPaths with successfully loaded images rather than non-empty paths.
Currently any non-empty path is added to tempExsitPaths, even when image loading/appendImage fails. This can produce doc names based on files that were never printed. Please only append to tempExsitPaths after the image has been successfully loaded/handled so the doc name matches the actual printed content.
Suggested implementation:
```cpp
m_re->appendImage(img);
if (!path.isEmpty()) {
tempExsitPaths << path;
}
}
}
}
```
Depending on the surrounding code, you may also want to ensure this block is only reached when the image has been successfully loaded (e.g. `!img.isNull()` or a successful return value from a loader). In that case, make sure the `if (!path.isEmpty()) { tempExsitPaths << path; }` remains inside that success-checking `if` so `tempExsitPaths` reflects only successfully loaded/printed images.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| if (!path.isEmpty()) { | ||
| tempExsitPaths << path; |
There was a problem hiding this comment.
suggestion (bug_risk): Align tempExsitPaths with successfully loaded images rather than non-empty paths.
Currently any non-empty path is added to tempExsitPaths, even when image loading/appendImage fails. This can produce doc names based on files that were never printed. Please only append to tempExsitPaths after the image has been successfully loaded/handled so the doc name matches the actual printed content.
Suggested implementation:
m_re->appendImage(img);
if (!path.isEmpty()) {
tempExsitPaths << path;
}
}
}
}
Depending on the surrounding code, you may also want to ensure this block is only reached when the image has been successfully loaded (e.g. !img.isNull() or a successful return value from a loader). In that case, make sure the if (!path.isEmpty()) { tempExsitPaths << path; } remains inside that success-checking if so tempExsitPaths reflects only successfully loaded/printed images.
Use the current image path and pass only its file name to the print dialog. 使用当前图片路径并仅向打印对话框传递文件名。 Log: 修复打印预览保存默认路径和文件名异常 Bug: https://pms.uniontech.com/bug-view-369191.html Influence: 保存 PDF 或图片时默认路径可用，并显示当前图片文件名。
Resurgamz
force-pushed
the
feature/fix-BUG369191-eagle
branch
from
d1ea8fa to
f50f78f
Compare
deepin pr auto review★ 总体评分：100分■ 【总体评价】
■ 【详细分析】
■ 【改进建议代码示例】 // SPDX-FileCopyrightText: 2020 - 2024 UnionTech Software Technology Co., Ltd.
//
// SPDX-License-Identifier: GPL-3.0-or-later
void PrintHelper::showPrintDialog(const QStringList &paths, QWidget *parent)
{
// ... 前置逻辑
if (!path.isEmpty()) {
tempExsitPaths << path;
}
// ... 前置逻辑
if (DApplication::runtimeDtkVersion() >= DTK_VERSION_CHECK(5, 4, 10, 0)) {
QString documentName = tempExsitPaths.isEmpty()
? QStringLiteral("print")
: QFileInfo(tempExsitPaths.first()).fileName();
if (documentName.isEmpty()) {
documentName = QStringLiteral("print");
}
printDialog2.setDocName(documentName);
}
} |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: max-lvs, Resurgamz The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
/forcemerge |
|
This pr force merged! (status: unstable) |
deepin-bot
Bot
merged commit a555336
into
linuxdeepin:develop/eagle
3 participants
Use the current image path and pass only its file name to the print dialog.
使用当前图片路径并仅向打印对话框传递文件名。
Log: 修复打印预览保存默认路径和文件名异常
Bug: https://pms.uniontech.com/bug-view-369191.html
Influence: 保存 PDF 或图片时默认路径可用，并显示当前图片文件名。
Summary by Sourcery
Bug Fixes: