From b24400b549337a1fc710ba9e4fb481bd9ee7c770 Mon Sep 17 00:00:00 2001 From: Roo Code Date: Mon, 21 Jul 2025 17:09:58 +0000 Subject: [PATCH] fix: address PR review feedback - Refactored showSuggestions from state variable to constant SHOW_SUGGESTIONS - Renamed breakingExps to stopPatterns for better clarity - Added test coverage for edge cases in command parsing - Fixed test assertion for multiline content handling --- .../src/components/chat/CommandExecution.tsx | 5 +- .../chat/__tests__/CommandExecution.spec.tsx | 51 +++++++++++++++++++ webview-ui/src/utils/commandPatterns.ts | 4 +- 3 files changed, 56 insertions(+), 4 deletions(-) diff --git a/webview-ui/src/components/chat/CommandExecution.tsx b/webview-ui/src/components/chat/CommandExecution.tsx index d8451c2ae6..38caaa62aa 100644 --- a/webview-ui/src/components/chat/CommandExecution.tsx +++ b/webview-ui/src/components/chat/CommandExecution.tsx @@ -50,7 +50,8 @@ export const CommandExecution = ({ executionId, text, icon, title }: CommandExec const [isExpanded, setIsExpanded] = useState(terminalShellIntegrationDisabled) const [streamingOutput, setStreamingOutput] = useState("") const [status, setStatus] = useState(null) - const showSuggestions = true + // Show suggestions is always enabled for command pattern management + const SHOW_SUGGESTIONS = true // The command's output can either come from the text associated with the // task message (this is the case for completed commands) or from the @@ -195,7 +196,7 @@ export const CommandExecution = ({ executionId, text, icon, title }: CommandExec - {showSuggestions && commandPatterns.length > 0 && ( + {SHOW_SUGGESTIONS && commandPatterns.length > 0 && ( { + // Test with a command that might cause parsing issues + const unparsableCommand = "echo 'test with unclosed quote" + + render( + + + , + ) + + // Should still render the command + expect(screen.getByTestId("code-block")).toHaveTextContent("echo 'test with unclosed quote") + + // Should show pattern selector with at least the main command + expect(screen.getByTestId("command-pattern-selector")).toBeInTheDocument() + expect(screen.getByText("echo")).toBeInTheDocument() + }) + + it("should handle empty or whitespace-only commands", () => { + render( + + + , + ) + + // Should render without errors + expect(screen.getByTestId("code-block")).toBeInTheDocument() + + // Should not show pattern selector for empty commands + expect(screen.queryByTestId("command-pattern-selector")).not.toBeInTheDocument() + }) + + it("should handle commands with only output and no command prefix", () => { + const outputOnly = `Some output without a command +Multiple lines of output +Without any command prefix` + + render( + + + , + ) + + // Should treat the entire text as command when no prefix is found + const codeBlock = screen.getByTestId("code-block") + // The mock CodeBlock component renders text content without preserving newlines + expect(codeBlock.textContent).toContain("Some output without a command") + expect(codeBlock.textContent).toContain("Multiple lines of output") + expect(codeBlock.textContent).toContain("Without any command prefix") + }) }) }) diff --git a/webview-ui/src/utils/commandPatterns.ts b/webview-ui/src/utils/commandPatterns.ts index 1dd2836ff0..23421c1bf0 100644 --- a/webview-ui/src/utils/commandPatterns.ts +++ b/webview-ui/src/utils/commandPatterns.ts @@ -52,12 +52,12 @@ function processCommand(cmd: any[], patterns: Set) { patterns.add(mainCmd) // Patterns that indicate we should stop looking for subcommands - const breakingExps = [/^-/, /[\\/.~ ]/] + const stopPatterns = [/^-/, /[\\/.~ ]/] // Build up patterns progressively for (let i = 1; i < cmd.length; i++) { const arg = cmd[i] - if (typeof arg !== "string" || breakingExps.some((re) => re.test(arg))) break + if (typeof arg !== "string" || stopPatterns.some((re) => re.test(arg))) break const pattern = cmd.slice(0, i + 1).join(" ") patterns.add(pattern)