fix: use state machine for argument parsing and support single quotes

[ComixHe]

• Replace boolean flag with SpliterState enum for better state management
• Add single quote support for GIO/Shell compatibility
• Improve escape character handling in double quotes
• Enhance error detection for unterminated quotes

Pms: BUG-351683

[sourcery-ai]

Sorry @ComixHe, you have reached your weekly rate limit of 500000 diff characters.

Please try again later or upgrade to continue using Sourcery

[BLumia]

要补一组测试么？

[ComixHe]

要补一组测试么？

补了

[deepin-ci-robot]

deepin pr auto review

这段代码主要对 ApplicationService::splitExecArguments 函数进行了重构，增加了对单引号的支持，并改进了状态机的实现。以下是对这段代码的详细审查和改进建议：

1. 语法逻辑审查

优点：

• 使用了 enum class SpliterState 替代了原来的布尔标志 inQuotes，使得状态管理更加清晰，逻辑更加严谨。
• 使用了 switch-case 结构处理不同状态，代码可读性显著提升。
• 正确处理了字符串末尾的反斜杠和未闭合的引号情况，返回 std::nullopt。

潜在问题与改进建议：

• 状态枚举命名：SpliterState 的拼写有误，应为 SplitterState（双 't'）。虽然不影响编译，但为了代码的专业性和可读性，建议修正。
• 迭代器越界检查：在处理反斜杠转义时，使用了 if (++it == str.end())。虽然逻辑正确，但直接修改迭代器 it 并在随后的 switch 结构外继续使用它，可能会让阅读者感到困惑。建议提取逻辑或增加注释说明。
• 错误信息缺失：在 SpliterState::Normal 分支中，当遇到末尾反斜杠时，直接返回 std::nullopt，没有像旧代码那样输出 qWarning。建议保留错误日志，便于调试：

  if (++it == str.end()) {
      qWarning() << "Exec parsing error: Backslash at end of line";
      return std::nullopt;
  }

2. 代码质量审查

优点：

• 代码结构清晰，状态机逻辑明确。
• 使用了 QStringView 作为输入参数，避免了不必要的字符串拷贝，性能较好。
• 使用了 noexcept 标记，表明该函数不会抛出异常。

改进建议：

• 代码注释：虽然状态机逻辑清晰，但建议在函数开头或关键逻辑处添加注释，解释各个状态的含义和转换条件，特别是单引号的处理逻辑（因为 Desktop Entry Spec 不支持单引号）。
• 魔法数字：uint8_t 用于枚举底层类型是合理的，但可以显式声明 enum class SpliterState : uint8_t，确保内存占用明确。
• 单引号处理：虽然为了兼容性增加了单引号支持，但建议在文档或注释中明确说明这是非标准行为，并提醒开发者谨慎使用。

3. 代码性能审查

优点：

• 使用了 QStringView 和 reserve，减少了内存分配和拷贝。
• 状态机逻辑高效，没有不必要的计算。

改进建议：

• 减少分支预测失败：switch-case 结构已经较好，但可以考虑将高频分支（如 Normal 状态）放在最前面，以优化分支预测。
• 避免频繁的 append 调用：在 Normal 状态下，连续的非空格字符可以一次性处理，而不是逐个字符 append。例如：

  auto nextSpace = std::find_if(it, str.end(), [](QChar c) { return c.isSpace() || c == u'\'' || c == u'"'; });
  currentToken.append(QStringView(it, nextSpace));
  it = nextSpace - 1; // 调整迭代器位置

  （注意：这种优化需要仔细处理迭代器位置，可能增加复杂度，需权衡收益。）

4. 代码安全审查

优点：

• 正确处理了边界条件（如末尾反斜杠、未闭合引号）。
• 使用了 noexcept，避免异常传播。

潜在风险与改进建议：

• 输入验证：虽然函数内部处理了空字符串，但建议在文档中明确说明输入应为合法的 Exec 字符串，避免调用者传入无效数据。
• 单引号的安全性：单引号的处理可能引入命令注入风险（如 sh -c '...; rm -rf /'）。建议在文档中警告开发者，或考虑限制单引号内的内容（如禁止嵌套引号或特殊字符）。
• 转义字符的一致性：双引号内的转义逻辑（如 \"、\$）与单引号不同（单引号内不处理转义）。建议在注释中明确说明，避免开发者混淆。

5. 测试用例审查

优点：

• 新增了单引号的测试用例，覆盖了兼容性需求。
• 包含了边界条件测试（如未闭合引号、末尾反斜杠）。

改进建议：

• 增加更多边界测试：例如：
  • 空字符串（已覆盖）。
  • 连续空格（如 "a b"）。
  • 混合引号（如 "a'b'c"）。
  • 转义字符测试（如 "\"a\""）。
• 单引号嵌套测试：虽然 Desktop Entry Spec 不支持，但可以测试 "'a'" 或 'a"b'c' 等情况，确保行为明确。

6. 其他建议

• 文档完善：建议为 splitExecArguments 函数添加 Doxygen 注释，说明其功能、参数、返回值及注意事项（如单引号是非标准支持）。
• 日志级别：qWarning 用于解析错误是合理的，但可以考虑在调试模式下输出更详细的上下文（如当前解析的字符串片段）。

总结

这段代码的重构整体质量较高，逻辑清晰，性能和安全方面也有较好的考虑。主要改进方向是：

1. 修正命名（如 SpliterState）。
2. 补充注释和文档。
3. 增加测试用例覆盖。
4. 考虑进一步优化性能（如减少 append 调用）。

[deepin-ci-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: BLumia, ComixHe

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

• debian/deepin/OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[ComixHe]

/force merge