feat: dde-am resolve command to full path before execution #328
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Reviewer's GuideThis PR updates dde-am command execution so that command names are resolved to full executable paths before invocation, and aborts execution early when the command is invalid or not found. Sequence diagram for command execution with full path resolutionsequenceDiagram
participant Main as handleExecuteCommand
participant Executor as CommandExecutor
participant QPaths as QStandardPaths
Main->>Executor: setProgram(program)
alt program is empty
Executor-->>Main: false
Main-->>Main: return -1
else program starts with slash
Executor-->>Main: true
Main->>Executor: setType(type)
Main->>Executor: setArguments(arguments)
Main-->>Main: proceed_with_execution
else program is relative
Executor->>QPaths: findExecutable(program)
alt fullPath is empty
QPaths-->>Executor: ""
Executor-->>Main: false
Main-->>Main: return -1
else fullPath found
QPaths-->>Executor: fullPath
Executor-->>Main: true
Main->>Executor: setType(type)
Main->>Executor: setArguments(arguments)
Main-->>Main: proceed_with_execution
end
end
Updated class diagram for CommandExecutor with resolving program pathclassDiagram
class CommandExecutor {
- QString m_program
+ bool setProgram(QString program)
+ void setArguments(QStringList arguments)
+ void setType(QString type)
+ void setRunId(QString runId)
}
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 left some high level feedback:
- Since
setProgramnow returnsbool, please double-check all existing call sites (including any outside this translation unit) to ensure they handle failure appropriately and that the changed signature doesn’t silently break other usages. - Currently only non-absolute commands are validated via
findExecutable; consider also checking that absolute paths actually point to an executable (e.g. withQFileInfo::isExecutable) to avoid attempting to run missing or non-executable files.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Since `setProgram` now returns `bool`, please double-check all existing call sites (including any outside this translation unit) to ensure they handle failure appropriately and that the changed signature doesn’t silently break other usages.
- Currently only non-absolute commands are validated via `findExecutable`; consider also checking that absolute paths actually point to an executable (e.g. with `QFileInfo::isExecutable`) to avoid attempting to run missing or non-executable files.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
When executing commands via dde-am, resolve relative command names to their full path using QStandardPaths::findExecutable. Log: dde-am resolve command to full path before execution Pms: BUG-349699
yixinshark
force-pushed
the
fix-ddeamfullcmdpath
branch
from
24c567a to
891122e
Compare
BLumia
approved these changes
Feb 3, 2026
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: BLumia, yixinshark 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 |
deepin pr auto review这段代码的修改主要目的是为了增强命令执行的健壮性，确保 1. 语法与逻辑审查
2. 代码质量与规范
3. 代码性能
4. 代码安全
综合改进建议针对 修改建议代码 ( #include <QCoreApplication> // 确保包含此头文件以使用 exit() 或返回码
#include <QTextStream> // 用于输出到控制台
#include <QDebug>
// ... (其他代码)
int handleExecuteCommand(const QCommandLineParser &parser,
const QCommandLineOption &executeOption,
const QCommandLineOption &typeOption,
const QCommandLineOption &envOption)
{
CommandExecutor executor;
QString cmd = parser.value(executeOption);
if (!executor.setProgram(cmd)) {
QTextStream(stderr) << "Error: Failed to execute command. "
<< "The executable '" << cmd << "' was not found or is invalid.\n";
return EXIT_FAILURE; // 使用标准宏定义通常比硬编码 -1 更好，或者保持 -1 也可以
}
if (parser.isSet(typeOption)) {
executor.setType(parser.value(typeOption));
}
// ... (后续代码)总结： |
Contributor
Author
|
/forcemerge |
|
This pr force merged! (status: blocked) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When executing commands via dde-am, resolve relative command names to their full path using QStandardPaths::findExecutable.
Log: dde-am resolve command to full path before execution
Pms: BUG-349699
Summary by Sourcery
Bug Fixes: