fix: Prevent overwriting error states on successful exit codes in Cli…#401
Conversation
…Interface - Updated the processFinished and extractProcessFinished methods to retain error states detected from stdout, even when the exit code is 0. - This change ensures that errors such as long file name issues are not masked by a successful exit code, improving error handling and user feedback. Enhances reliability in archive processing by maintaining accurate error reporting. bug: https://pms.uniontech.com/bug-view-338535.html
Reviewer's guide (collapsed on small PRs)Reviewer's GuideAdjusts CLI archive interface completion handling so that error states detected from stdout are preserved and not overwritten when the underlying CLI tool exits with code 0, improving accuracy of extraction/processing results. Sequence diagram for updated CLI process finish handlingsequenceDiagram
participant QProcess
participant CliInterface
participant StdoutParser
QProcess->>CliInterface: readyReadStandardOutput()
CliInterface->>StdoutParser: parseStdout(output)
StdoutParser-->>CliInterface: detectedError
CliInterface->>CliInterface: set m_finishType = PFT_Error
QProcess->>CliInterface: finished(exitCode = 0, exitStatus)
CliInterface->>CliInterface: processFinished(exitCode, exitStatus)
alt exitCode == 0 and m_finishType != PFT_Error and m_finishType != PFT_Cancel
CliInterface->>CliInterface: m_finishType = PFT_Nomral
else existing error or cancel state
CliInterface->>CliInterface: keep current m_finishType
end
Updated class diagram for CliInterface finish type handlingclassDiagram
class CliInterface {
- bool m_isCorruptArchive
- FinishType m_finishType
+ void processFinished(int exitCode, QProcess_ExitStatus exitStatus)
+ void extractProcessFinished(int exitCode, QProcess_ExitStatus exitStatus)
}
class FinishType {
<<enumeration>>
PFT_Nomral
PFT_Error
PFT_Cancel
}
CliInterface --> FinishType : uses
class QProcess_ExitStatus {
<<enumeration>>
NormalExit
CrashExit
}
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
deepin pr auto review这段代码的修改主要是为了处理命令行工具(CLI)返回状态码与实际执行状态不一致的情况,特别是在处理某些归档工具(如 以下是对该 diff 的详细审查意见: 1. 语法逻辑审查现状: if (exitCode == 0 && m_finishType != PFT_Error && m_finishType != PFT_Cancel) {
m_finishType = PFT_Nomral;
}分析:
2. 代码质量审查
3. 代码性能审查
4. 代码安全审查
改进建议
总结这个 diff 是一个高质量的修复,主要针对边缘情况(CLI 工具误报成功)进行了处理,显著提升了软件的可靠性。除了需要确认常量名的拼写外,没有明显的逻辑或安全隐患。 |
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- The success-handling condition (
exitCode == 0 && m_finishType != PFT_Error && m_finishType != PFT_Cancel) is now duplicated in bothprocessFinishedandextractProcessFinished; consider extracting this into a small helper (e.g.setFinishedOnSuccess(exitCode)) to keep the success/error logic consistent in one place.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The success-handling condition (`exitCode == 0 && m_finishType != PFT_Error && m_finishType != PFT_Cancel`) is now duplicated in both `processFinished` and `extractProcessFinished`; consider extracting this into a small helper (e.g. `setFinishedOnSuccess(exitCode)`) to keep the success/error logic consistent in one place.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: dengzhongyuan365-dev, lzwind 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 |
|
/merge |
…Interface
Enhances reliability in archive processing by maintaining accurate error reporting.
bug: https://pms.uniontech.com/bug-view-338535.html
Summary by Sourcery
Bug Fixes: