Skip to content

Conversation

@add-uos
Copy link
Contributor

@add-uos add-uos commented Jan 20, 2026

When saving files with the default filename, the code now automatically appends the appropriate file extension based on the selected file filter. This prevents saving files without extensions and ensures proper file association.

log: ensure default filename has correct extension
bug: https://pms.uniontech.com/bug-view-331447.html

Summary by Sourcery

Ensure the file save dialog applies a proper extension to the default filename based on the selected file filter.

Bug Fixes:

  • Prevent saving files with a default name that lacks the expected file extension by appending the suffix from the active file filter.

Enhancements:

  • Expose the file dialog suffix-extraction helper so it can be reused when preparing the default filename.

When saving files with the default filename, the code now automatically
appends the appropriate file extension based on the selected file filter.
This prevents saving files without extensions and ensures proper file
association.

log: ensure default filename has correct extension
bug: https://pms.uniontech.com/bug-view-331447.html
@sourcery-ai
Copy link

sourcery-ai bot commented Jan 20, 2026

Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

Ensures the default filename used in the file save dialog has the correct extension derived from the active file filter, and exposes the suffix-extraction helper on FileSelectDialog for this purpose.

Sequence diagram for default filename extension handling in file save dialog

sequenceDiagram
    actor User
    participant DrawBoard
    participant DrawBoard_private
    participant FileSelectDialog
    participant DrawApp

    User->>DrawBoard: triggerSave()
    DrawBoard->>DrawBoard_private: execFileSelectDialog(defaultFileName, toddf, file)
    alt toddf is true
        DrawBoard_private->>FileSelectDialog: construct with _borad as parent
        DrawBoard_private->>FileSelectDialog: setAcceptMode(Save)
        DrawBoard_private->>FileSelectDialog: setNameFilters(...)
        alt file is not empty
            DrawBoard_private->>FileSelectDialog: selectFile(file)
            DrawBoard_private->>FileSelectDialog: setDirectory(dir of file)
        else file is empty
            DrawBoard_private->>FileSelectDialog: fullFileName = defaultFileName
            DrawBoard_private->>FileSelectDialog: fileInfo = QFileInfo(fullFileName)
            alt fileInfo.suffix is empty
                DrawBoard_private->>DrawApp: defaultFileDialogNameFilter()
                DrawApp-->>DrawBoard_private: filter
                DrawBoard_private->>FileSelectDialog: extractSuffix(filter)
                FileSelectDialog-->>DrawBoard_private: suffix
                alt suffix not empty
                    DrawBoard_private->>DrawBoard_private: normalize suffix with leading dot
                    DrawBoard_private->>DrawBoard_private: append suffix if missing
                end
            end
            DrawBoard_private->>FileSelectDialog: selectFile(fullFileName)
            DrawBoard_private->>FileSelectDialog: setDirectory(drawApp.defaultFileDialogPath())
        end
        DrawBoard_private->>FileSelectDialog: exec()
        FileSelectDialog-->>DrawBoard_private: dialog result
        DrawBoard_private-->>DrawBoard: selected file path
    else toddf is false
        DrawBoard_private-->>DrawBoard: use alternative path selection
    end
    DrawBoard-->>User: file saved with correct extension
Loading

Updated class diagram for FileSelectDialog and DrawBoard_private

classDiagram
    class FileSelectDialog {
        +FileSelectDialog(QWidget parent)
        +int exec()
        +QString resultFile() const
        +QString extractSuffix(QString filter)
        -void saveSetting()
        -QString checkAndBuildPath(QString path)
        -QString _resultFile
    }

    class DrawBoard_private {
        +QString execFileSelectDialog(QString defaultFileName, bool toddf, QString file)
        -QWidget _borad
    }

    class DrawBoard {
        -DrawBoard_private* d
        +void triggerSave()
    }

    DrawBoard --> DrawBoard_private : uses
    DrawBoard_private --> FileSelectDialog : creates
    DrawBoard_private --> DrawBoard : enclosing_class_relationship
Loading

File-Level Changes

Change Details Files
Ensure default filename in save dialog automatically gets an appropriate extension based on the selected file filter.
  • Renamed execFileSelectDialog parameter from a misspelled name to defaultFileName for clarity.
  • When no explicit file is provided, build a fullFileName from the defaultFileName and append an extension if it has no suffix.
  • Use FileSelectDialog::extractSuffix with drawApp->defaultFileDialogNameFilter() to determine the extension, normalizing a missing leading dot and avoiding duplicate suffixes.
  • Pass the computed fullFileName into dialog.selectFile and keep using the default file dialog path as the directory.
src/frame/ccentralwidget.cpp
Make the suffix-extraction helper publicly accessible on FileSelectDialog so it can be used by callers.
  • Moved extractSuffix declaration from the private to the public section of FileSelectDialog.
  • Kept other private helper declarations (saveSetting, checkAndBuildPath) unchanged.
src/frame/cmultiptabbarwidget.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

deepin pr auto review

这段代码的修改主要是为了在文件对话框打开时,自动为没有后缀名的默认文件名添加后缀。以下是对这段代码的详细审查和改进建议:

1. 语法与逻辑审查

  • 拼写修正
    • 将参数名 defualFileName 修正为 defaultFileName,修正了拼写错误,符合变量命名规范。
  • 逻辑实现
    • 新增的逻辑旨在检测 defaultFileName 是否包含后缀。如果不包含,则从 drawApp 获取默认的文件过滤器,提取后缀(如 .ddf),并将其追加到文件名后。
    • 逻辑流程清晰:检查后缀 -> 提取后缀 -> 格式化后缀(加点) -> 拼接。
    • FileSelectDialog 类中,将 extractSuffix 函数从 private 移至 public,以便 DrawBoard_private 能够调用,这是必要的修改。

2. 代码质量

  • 建议:使用 const 引用传递 QString
    • FileSelectDialog 类中,extractSuffix(const QString &filter) 已经使用了 const 引用,这是很好的做法。
    • DrawBoard_private 中,execFileSelectDialog 的参数 QString file = "" 建议改为 const QString &file = QString()const QString &file = ""。虽然 QString 隐式共享且拷贝开销小,但在高频调用或作为接口设计时,使用 const 引用是更标准的 C++ 写法,能明确表达“不修改原数据”的意图。
  • 建议:字符串拼接
    • fullFileName = fullFileName + suffix; 可以改为 fullFileName.append(suffix); 或者 fullFileName += suffix;,语义更直接,且通常 QString+= 有优化。

3. 代码性能

  • QFileInfo 的使用
    • QFileInfo fileInfo(fullFileName); 构造函数会进行文件系统查询(如果路径存在)。虽然这里主要是为了获取后缀,开销尚可接受,但需注意如果 fullFileName 是一个非常长的网络路径或无效路径,可能会有轻微性能损耗。
  • 重复计算
    • drawApp->defaultFileDialogNameFilter() 可能会在每次调用时进行字符串构造或配置查询。如果该函数内部没有缓存,建议在外部缓存该结果,或者确保 drawApp 内部已经做了缓存处理。

4. 代码安全

  • 后缀处理的安全性
    • 代码中检查了 suffix.startsWith(".") 并自动添加,这很好,防止了出现 fileext 而不是 .fileext 的情况。
    • checkAndBuildPathextractSuffix 的具体实现未展示,但需确保 extractSuffix 能够正确处理复杂的过滤器字符串(例如 "Images (*.png *.jpg)"),确保提取的是第一个后缀或者是期望的后缀。
  • 空指针/异常
    • 代码假设 drawApp 指针有效。如果 drawApp 可能为空,需要添加判空保护(例如 if (drawApp) ...)。
    • dialog.setDirectory(QFileInfo(file).dir().absolutePath()); 这行代码中,如果 file 是无效路径,QFileInfo 处理通常比较健壮,但最好确认一下 file 是否为空或仅仅是文件名,避免设置到错误的目录。

5. 改进后的代码示例

结合以上建议,优化后的代码片段如下:

// src/frame/ccentralwidget.cpp
// ...

// 建议参数使用 const 引用
QString execFileSelectDialog(const QString &defaultFileName, bool toddf = true, const QString &file = QString())
{
    if (toddf) {
        FileSelectDialog dialog(_borad);
        // ... (其他初始化代码)

        if (!file.isEmpty()) {
            dialog.selectFile(file);
            dialog.setDirectory(QFileInfo(file).dir().absolutePath());
        } else {
            // Add a format suffix to the default filename
            QString fullFileName = defaultFileName;
            QFileInfo fileInfo(fullFileName);
            
            if (fileInfo.suffix().isEmpty()) {
                // 建议确保 drawApp 有效
                if (drawApp) {
                    QString nameFilter = drawApp->defaultFileDialogNameFilter();
                    QString suffix = dialog.extractSuffix(nameFilter);
                    
                    if (!suffix.isEmpty()) {
                        if (!suffix.startsWith("."))
                            suffix.prepend(".");
                        // 使用 += 替代 +
                        if (!fullFileName.endsWith(suffix))
                            fullFileName += suffix;
                    }
                }
            }
            dialog.selectFile(fullFileName);
            dialog.setDirectory(drawApp ? drawApp->defaultFileDialogPath() : QString());
        }
        dialog.exec();
        // ...
    }
    // ...
}

总结

这段修改解决了默认文件名缺少后缀的问题,逻辑正确。主要的改进点在于:

  1. file 参数改为 const QString & 以符合现代 C++ 最佳实践。
  2. 增加对 drawApp 指针的判空保护,增强健壮性。
  3. 使用 += 进行字符串拼接,提高可读性。

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 that normalizes and appends the suffix to defaultFileName would be clearer and more reusable if it were encapsulated inside FileSelectDialog (e.g., a helper like buildDefaultFileNameWithSuffix(...)) instead of reimplementing the suffix handling in execFileSelectDialog.
  • When appending the suffix, consider normalizing against the filter pattern rather than simple string checks (startsWith('.'), endsWith(suffix)), as this can behave unexpectedly with multi-part extensions or filters containing multiple patterns (e.g., "*.jpg *.jpeg").
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- The logic that normalizes and appends the suffix to `defaultFileName` would be clearer and more reusable if it were encapsulated inside `FileSelectDialog` (e.g., a helper like `buildDefaultFileNameWithSuffix(...)`) instead of reimplementing the suffix handling in `execFileSelectDialog`.
- When appending the suffix, consider normalizing against the filter pattern rather than simple string checks (`startsWith('.')`, `endsWith(suffix)`), as this can behave unexpectedly with multi-part extensions or filters containing multiple patterns (e.g., `"*.jpg *.jpeg"`).

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

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: add-uos, 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

@add-uos
Copy link
Contributor Author

add-uos commented Jan 20, 2026

/merge

@deepin-bot deepin-bot bot merged commit a9c1a3a into linuxdeepin:develop/snipe Jan 20, 2026
18 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