Skip to content

Commit ad35b4f

Browse files
committed
fix: properly handle multi-line commands in quotes
- Fix parseCommand to preserve newlines within quoted strings instead of incorrectly splitting them - Add UI filtering to exclude multi-line patterns from command pattern selector - Multi-line git commit messages now work correctly for auto-approval validation - UI shows clean patterns like 'git' and 'git commit' instead of unwieldy full commit text - Add comprehensive tests for multi-line command handling - Much simpler approach than placeholder substitution in PR #8792 Fixes the issue where multi-line git commit messages were being split into separate commands and causing cluttered UI in the auto-approved commands list.
1 parent 39448f4 commit ad35b4f

File tree

4 files changed

+93
-15
lines changed

4 files changed

+93
-15
lines changed

webview-ui/src/components/chat/CommandExecution.tsx

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -63,8 +63,10 @@ export const CommandExecution = ({ executionId, text, icon, title }: CommandExec
6363

6464
// Add all individual commands first
6565
allCommands.forEach((cmd) => {
66-
if (cmd.trim()) {
67-
allPatterns.add(cmd.trim())
66+
const trimmed = cmd.trim()
67+
// Skip patterns containing newlines - these are multi-line and not useful for UI patterns
68+
if (trimmed && !trimmed.includes("\n") && !trimmed.includes("\r")) {
69+
allPatterns.add(trimmed)
6870
}
6971
})
7072

webview-ui/src/components/chat/__tests__/CommandExecution.spec.tsx

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -564,5 +564,37 @@ Output:
564564
expect(codeBlocks.length).toBeGreaterThan(1)
565565
expect(codeBlocks[1]).toHaveTextContent("0 total")
566566
})
567+
568+
it("should filter out multi-line patterns from command pattern selector", () => {
569+
// Test with a multi-line git commit command similar to the PR issue
570+
const multiLineGitCommand = `git commit -m "feat: title
571+
572+
- point a
573+
- point b"`
574+
575+
render(
576+
<ExtensionStateWrapper>
577+
<CommandExecution executionId="test-18" text={multiLineGitCommand} />
578+
</ExtensionStateWrapper>,
579+
)
580+
581+
// Should show pattern selector
582+
const selector = screen.getByTestId("command-pattern-selector")
583+
expect(selector).toBeInTheDocument()
584+
585+
// Should show useful single-line patterns like "git" and "git commit"
586+
expect(selector.textContent).toMatch(/git/)
587+
588+
// Should NOT show the full collapsed multi-line content (which would be very long)
589+
// The pattern selector should contain short, useful patterns only
590+
const patterns = selector.textContent || ""
591+
// Check that no pattern is suspiciously long (> 50 chars would indicate collapsed multi-line content)
592+
const longPatternMatch = patterns.match(/\b\S{51,}\b/)
593+
expect(longPatternMatch).toBeNull()
594+
595+
// But the original command should still be shown in the code block
596+
const codeBlock = screen.getByTestId("code-block")
597+
expect(codeBlock.textContent).toContain("feat: title")
598+
})
567599
})
568600
})

webview-ui/src/utils/__tests__/command-validation.spec.ts

Lines changed: 32 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -110,15 +110,40 @@ describe("Command Validation", () => {
110110
])
111111
})
112112

113-
it("splits on actual newlines even within quotes", () => {
114-
// Note: Since we split by newlines first, actual newlines in the input
115-
// will split the command, even if they appear to be within quotes
113+
it("preserves newlines within quoted strings", () => {
114+
// Newlines inside quoted strings should be preserved as part of the command
116115
// Using template literal to create actual newline
117116
const commandWithNewlineInQuotes = `echo "Hello
118-
World"
119-
git status`
120-
// The quotes get stripped because they're no longer properly paired after splitting
121-
expect(parseCommand(commandWithNewlineInQuotes)).toEqual(["echo Hello", "World", "git status"])
117+
World"
118+
git status`
119+
// The newlines inside quotes are preserved, so we get two commands
120+
expect(parseCommand(commandWithNewlineInQuotes)).toEqual(['echo "Hello\nWorld"', "git status"])
121+
})
122+
123+
it("handles multi-line git commit messages correctly", () => {
124+
// Real-world case: multi-line git commit messages in quotes
125+
const multiLineCommit = `git commit -m "feat: add new feature
126+
127+
- Point A
128+
- Point B"
129+
git status`
130+
// The multi-line commit message should be preserved as one command
131+
expect(parseCommand(multiLineCommit)).toEqual([
132+
`git commit -m "feat: add new feature\n\n- Point A\n- Point B"`,
133+
"git status",
134+
])
135+
})
136+
137+
it("handles mixed single and double quotes with newlines", () => {
138+
const mixedQuotes = `echo 'single line'
139+
echo "multi
140+
line"
141+
echo 'another single'`
142+
expect(parseCommand(mixedQuotes)).toEqual([
143+
"echo 'single line'",
144+
'echo "multi\nline"',
145+
"echo 'another single'",
146+
])
122147
})
123148

124149
it("handles quoted strings on single line", () => {

webview-ui/src/utils/command-validation.ts

Lines changed: 25 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -127,26 +127,45 @@ export function containsDangerousSubstitution(source: string): boolean {
127127
* chaining operators (&&, ||, ;, |, or &) and newlines.
128128
*
129129
* Uses shell-quote to properly handle:
130-
* - Quoted strings (preserves quotes)
130+
* - Quoted strings (preserves quotes and newlines within quotes)
131131
* - Subshell commands ($(cmd), `cmd`, <(cmd), >(cmd))
132132
* - PowerShell redirections (2>&1)
133133
* - Chain operators (&&, ||, ;, |, &)
134-
* - Newlines as command separators
134+
* - Newlines as command separators (but not within quotes)
135135
*/
136136
export function parseCommand(command: string): string[] {
137137
if (!command?.trim()) return []
138138

139-
// Split by newlines first (handle different line ending formats)
140-
// This regex splits on \r\n (Windows), \n (Unix), or \r (old Mac)
141-
const lines = command.split(/\r\n|\r|\n/)
139+
// First protect quoted strings to avoid splitting on newlines inside quotes
140+
const quotes: string[] = []
141+
let protectedCommand = command
142+
143+
// Protect double-quoted strings (including multi-line) - simpler regex
144+
protectedCommand = protectedCommand.replace(/"[^"]*"/gs, (match) => {
145+
quotes.push(match)
146+
return `__QUOTE_${quotes.length - 1}__`
147+
})
148+
149+
// Protect single-quoted strings (including multi-line) - simpler regex
150+
protectedCommand = protectedCommand.replace(/'[^']*'/gs, (match) => {
151+
quotes.push(match)
152+
return `__QUOTE_${quotes.length - 1}__`
153+
})
154+
155+
// Now split by newlines (only unquoted newlines will be split)
156+
const lines = protectedCommand.split(/\r\n|\r|\n/)
142157
const allCommands: string[] = []
143158

144159
for (const line of lines) {
145160
// Skip empty lines
146161
if (!line.trim()) continue
147162

163+
// Restore quotes in this line before processing
164+
let restoredLine = line
165+
restoredLine = restoredLine.replace(/__QUOTE_(\d+)__/g, (_, i) => quotes[parseInt(i)])
166+
148167
// Process each line through the existing parsing logic
149-
const lineCommands = parseCommandLine(line)
168+
const lineCommands = parseCommandLine(restoredLine)
150169
allCommands.push(...lineCommands)
151170
}
152171

0 commit comments

Comments
 (0)