Skip to content

Conversation

@lichaofan2008
Copy link

@lichaofan2008 lichaofan2008 commented Jan 22, 2026

For the "Save As" module, there should be corresponding error prompts for unsupported file formats. For the export module, unsupported file formats should not be displayed to avoid misleading users.
另存为模块,不支持的文件格式要有相应的错误提示。导出模块,不支持的文件格式不显示出来,避免误导用户。

Bug: https://pms.uniontech.com/bug-view-328141.html

Summary by Sourcery

Handle unsupported image formats consistently in save and export flows.

Bug Fixes:

  • Show an error message when saving images with unsupported formats fails in the Save As workflow.
  • Hide unsupported AVIF and HEIC formats from the export format selector to avoid misleading users.
  • Guard export dialog format selection against out-of-range indices when restoring the previous format.

For the "Save As" module, there should be corresponding error prompts for unsupported file formats. For the export module, unsupported file formats should not be displayed to avoid misleading users.
另存为模块,不支持的文件格式要有相应的错误提示。导出模块,不支持的文件格式不显示出来,避免误导用户。

Bug: https://pms.uniontech.com/bug-view-328141.html
@sourcery-ai
Copy link

sourcery-ai bot commented Jan 22, 2026

Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

Adds explicit error handling for unsupported image formats when saving, and hides unsupported formats from the export dialog while making the format combobox index selection more robust, including a new localized error message.

Sequence diagram for unsupported format handling in Save As

sequenceDiagram
    actor User
    participant SaveAsDialog
    participant FileHander
    participant FileHanderPrivate
    participant QImage
    participant ErrorDialog

    User->>SaveAsDialog: clickSaveAs(fileWithUnsupportedExt)
    SaveAsDialog->>FileHander: saveToImage(context, file, format)
    FileHander->>QImage: save(file, format, imageQuility)
    QImage-->>FileHander: saveResult(false)
    FileHander->>FileHander: log qWarning
    FileHander->>FileHanderPrivate: setError(EUnSupportFile, localizedMessage)
    FileHanderPrivate-->>FileHander: errorSet
    FileHander-->>SaveAsDialog: return false
    SaveAsDialog->>ErrorDialog: show(localizedMessage)
    ErrorDialog-->>User: displayUnsupportedFormatError
Loading

Updated class diagram for file saving and export image dialog

classDiagram
    class FileHander {
        +bool saveToImage(PageContext* context, QString file, QString stuff)
        -void logSaveFailure(QString file)
    }

    class FileHanderPrivate {
        +void setError(int errorCode, QString message)
    }

    class PageContext

    class CExportImageDialog {
        -DComboBox* m_formatCombox
        -DSlider* m_qualitySlider
        +void initUI()
        +void showEvent(QShowEvent* event)
        -void removeUnsupportedFormats()
        -void adjustFormatIndex(QString formatFilter)
    }

    class DrawApp {
        +QStringList writableFormatNameFilters()
    }

    class DComboBox {
        +void addItems(QStringList items)
        +int count()
    }

    class DSlider

    class QShowEvent

    FileHander --> FileHanderPrivate : uses
    CExportImageDialog --> DrawApp : uses
    CExportImageDialog --> DComboBox : owns
    CExportImageDialog --> DSlider : owns
    CExportImageDialog --> QShowEvent : overrides
Loading

File-Level Changes

Change Details Files
Add error reporting when image saving fails due to unsupported file format.
  • Wrap the non-QImageWriter save path in a block that captures the boolean result of image.save().
  • On failure, log a warning with the target file path.
  • Set a specific EUnSupportFile error with a user-facing message indicating unsupported file format.
  • Return the actual save result instead of assuming success.
src/service/filehander.cpp
Hide unsupported image formats from the export dialog and guard against out-of-range format indexes.
  • Filter out AVIF and HEIC/HEIF formats from the writable format filters list before populating the combo box.
  • After mapping the current format to an index, clamp the index to the combo box item count to avoid out-of-range access, defaulting to 0 when necessary.
src/widgets/dialog/cexportimagedialog.cpp
Add translation for the new unsupported file format error message.
  • Introduce a zh_CN translation string for the new 'Unable to save "%1", unsupported file format' message in filehander.cpp.
translations/deepin-draw_zh_CN.ts

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

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:

  • The logic for hiding unsupported formats in the export dialog (hard-coded removal of AVIF/HEIC) is now duplicated/special-cased; consider centralizing the list or a helper that both writableFormatNameFilters() and the UI use so the supported/unsupported formats stay consistent in one place.
  • In CExportImageDialog::showEvent, the index is derived from the full writableFormatNameFilters() list but applied to the combobox that has had some formats removed; it would be more robust to compute the index against the combobox’s actual items (or a pre-filtered list) instead of clamping when out of range.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- The logic for hiding unsupported formats in the export dialog (hard-coded removal of AVIF/HEIC) is now duplicated/special-cased; consider centralizing the list or a helper that both `writableFormatNameFilters()` and the UI use so the supported/unsupported formats stay consistent in one place.
- In `CExportImageDialog::showEvent`, the index is derived from the full `writableFormatNameFilters()` list but applied to the combobox that has had some formats removed; it would be more robust to compute the index against the combobox’s actual items (or a pre-filtered list) instead of clamping when out of range.

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

deepin pr auto review

这段代码的改动主要涉及图像保存功能的错误处理、UI层对导出格式列表的过滤以及国际化文本的添加。整体来看,代码逻辑是正确的,能够解决潜在的用户体验问题(如保存失败无提示、选择不支持的格式)。

以下是从语法逻辑、代码质量、代码性能和代码安全四个方面提出的详细审查和改进意见:

1. 语法逻辑

  • filehander.cpp:

    • 逻辑正确性:在 image.save 失败时增加了日志输出 qWarning 和错误状态设置 setError,逻辑是完整的。这比原先直接返回 false 且无任何提示要好得多。
    • 拼写建议:变量名 imageQuility 拼写有误,建议修改为 imageQuality。虽然这不影响编译,但会影响代码可读性。
    • QImageWriter vs QImage::save:代码中使用了两种保存方式。QImageWriterwrite() 返回 false 时,也可以调用 w.errorString() 获取更详细的错误原因,建议在 else 分支中也考虑获取或利用 Qt 的错误字符串机制,以便于调试。
  • cexportimagedialog.cpp:

    • 索引边界检查:在 showEvent 中增加了 if (index >= m_formatCombox->count()) 的检查,这是非常必要的。因为之前移除了 AVIF 和 HEIC 格式,如果用户上次保存的是这两种格式之一,计算出的 index 可能会越界。这个修复逻辑严密。
    • 硬编码字符串风险:代码中使用了硬编码字符串 writeableFormats.removeAll("AVIF(*.avif)")。如果 Qt 版本升级或者底层库返回的字符串格式发生变化(例如大小写变化,或者括号前有空格),这段代码可能会失效。建议确认 drawApp->writableFormatNameFilters() 返回的字符串格式是否严格固定。

2. 代码质量

  • 可维护性与硬编码

    • cexportimagedialog.cpp 中,通过字符串匹配来移除格式比较脆弱。
    • 改进建议:如果可能,建议在 drawApp 或配置层维护一个"黑名单"或"白名单"过滤器,而不是在 UI 初始化代码中硬编码移除特定的字符串。或者,检查 Qt 的 QImageWriter::supportedImageFormats() 是否能更可靠地判断支持情况。
  • 错误处理的一致性

    • filehander.cpp 中对 QImageWriter 的处理(if 分支)没有错误提示,而对 QImage::saveelse 分支)增加了错误提示。
    • 改进建议:为了保持一致性,建议在 QImageWriter 失败时也添加类似的 qWarningsetError 处理。

3. 代码性能

  • 性能影响
    • 这些修改主要发生在文件保存和 UI 初始化阶段,涉及的操作(字符串比较、列表移除)开销非常小,不会对性能产生负面影响。
    • QImage::save 本身是 I/O 密集型操作,增加的布尔判断和日志记录相对于 I/O 耗时可以忽略不计。

4. 代码安全

  • 文件路径安全
    • file 变量直接传入了 image.save 和日志输出。如果 file 包含特殊字符或非常长的路径,日志输出可能会造成控制台显示混乱,但 Qt 的文件保存函数通常会处理路径安全性。
    • 改进建议:在日志输出时,可以适当截断过长的文件名,或者确保 file 来源可信。
  • 国际化字符串
    • 新增的翻译字符串 "Unable to save \"%1\", unsupported file format" 使用了 %1 占位符,这是安全的。翻译文件 deepin-draw_zh_CN.ts 的更新也是正确的。

总结与代码优化示例

代码整体改动是正向的,主要是增强了健壮性。以下是针对 filehander.cpp 的优化建议代码,旨在统一错误处理并修正拼写:

// 建议优化后的 filehander.cpp 片段
bool FileHander::saveToImage(PageContext *context, const QString &file, const QString &stuff, int imageQuality) // 建议修正参数名 imageQuility -> imageQuality
{
    // ...前置代码保持不变...

    if (image.isNull()) {
        return false;
    } else {
        // 如果是 JPEG,使用 QImageWriter 以便更好地控制压缩比
        if (stuff.contains("jpeg", Qt::CaseInsensitive) || stuff.contains("jpg", Qt::CaseInsensitive)) {
            QImageWriter w(file, stuff.toLocal8Bit());
            w.setCompression(1);
            bool ret = w.write(image);
            if (!ret) {
                qWarning() << "Save image (Writer):" << file << "FAILED! Error:" << w.errorString();
                d_pri()->setError(EUnSupportFile, tr("Unable to save \"%1\", unsupported file format").arg(info.fileName()));
            }
            return ret;
        } else {
            bool ret = image.save(file, stuff.toLocal8Bit(), imageQuality);
            if (!ret) {
                qWarning() << "Save image (Default):" << file << "FAILED!";
                d_pri()->setError(EUnSupportFile, tr("Unable to save \"%1\", unsupported file format").arg(info.fileName()));
            }
            return ret;
        }
    }
    return false;
}

针对 cexportimagedialog.cpp 的优化建议

如果不想硬编码字符串,可以尝试检查后缀名,而不是完整的过滤器字符串:

// 示例:通过后缀名过滤,稍微灵活一些
QStringList unwantedSuffixes = {"avif", "heic", "heif"};
// 使用 removeIf (C++17/Qt 5.14+) 或手动遍历
for (int i = writeableFormats.size() - 1; i >= 0; --i) {
    const QString &fmt = writeableFormats[i];
    bool shouldRemove = false;
    for (const QString &suffix : unwantedSuffixes) {
        if (fmt.contains(suffix, Qt::CaseInsensitive)) {
            shouldRemove = true;
            break;
        }
    }
    if (shouldRemove) {
        writeableFormats.removeAt(i);
    }
}

不过,考虑到 writableFormatNameFilters 通常返回格式固定的字符串(如 "JPEG (*.jpg *.jpeg)"),原始的硬编码修改在当前环境下也是可以接受的,只要确保 Qt 版本兼容即可。

@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
Author

/forcemerge

@deepin-bot
Copy link
Contributor

deepin-bot bot commented Jan 22, 2026

This pr force merged! (status: unstable)

@deepin-bot deepin-bot bot merged commit 2abfcab into linuxdeepin:release/eagle Jan 22, 2026
17 of 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