Skip to content

Commit 56234ce

Browse files
committed
Fix code duplication issues in streaming diff animation
- Eliminated duplicate temp file creation and diff view opening logic in StreamingDiffController - Consolidated session creation logic in DiffAnimationHandler - Created reusable cleanup helper function in Messages - Removed duplicate getCursorState function - Reduced codebase by 53+ lines of duplicate code
1 parent dd5e4c8 commit 56234ce

File tree

3 files changed

+90
-99
lines changed

3 files changed

+90
-99
lines changed

packages/amazonq/src/lsp/chat/diffAnimation/diffAnimationHandler.ts

Lines changed: 21 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -252,21 +252,28 @@ export class DiffAnimationHandler implements vscode.Disposable {
252252
}
253253
}
254254

255-
public async startStreamingDiffSession(toolUseId: string, filePath: string): Promise<void> {
255+
public async startStreamingDiffSession(
256+
toolUseId: string,
257+
filePath: string,
258+
providedOriginalContent?: string
259+
): Promise<void> {
256260
try {
257-
let originalContent = ''
258-
const pendingWrite = Array.from(this.pendingWrites.values()).find(
259-
(write) => write.toolUseId === toolUseId && write.filePath === filePath
260-
)
261+
let originalContent = providedOriginalContent || ''
261262

262-
if (pendingWrite) {
263-
originalContent = pendingWrite.originalContent
264-
} else {
265-
try {
266-
const document = await vscode.workspace.openTextDocument(vscode.Uri.file(filePath))
267-
originalContent = document.getText()
268-
} catch {
269-
originalContent = ''
263+
if (!providedOriginalContent) {
264+
const pendingWrite = Array.from(this.pendingWrites.values()).find(
265+
(write) => write.toolUseId === toolUseId && write.filePath === filePath
266+
)
267+
268+
if (pendingWrite) {
269+
originalContent = pendingWrite.originalContent
270+
} else {
271+
try {
272+
const document = await vscode.workspace.openTextDocument(vscode.Uri.file(filePath))
273+
originalContent = document.getText()
274+
} catch {
275+
originalContent = ''
276+
}
270277
}
271278
}
272279

@@ -288,17 +295,7 @@ export class DiffAnimationHandler implements vscode.Disposable {
288295
filePath: string,
289296
originalContent: string
290297
): Promise<void> {
291-
try {
292-
this.streamingSessions.set(toolUseId, {
293-
toolUseId,
294-
filePath,
295-
originalContent,
296-
startTime: Date.now(),
297-
})
298-
await this.streamingDiffController.openStreamingDiffView(toolUseId, filePath, originalContent)
299-
} catch (error) {
300-
getLogger().error(`[DiffAnimationHandler] ❌ Failed to start streaming session for ${toolUseId}: ${error}`)
301-
}
298+
return this.startStreamingDiffSession(toolUseId, filePath, originalContent)
302299
}
303300

304301
public async streamContentUpdate(

packages/amazonq/src/lsp/chat/diffAnimation/streamingDiffController.ts

Lines changed: 52 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -376,6 +376,51 @@ export class StreamingDiffController implements vscode.Disposable {
376376
}
377377
}
378378

379+
/**
380+
* Helper method to create temp file and open diff view - eliminates code duplication
381+
*/
382+
private async createTempFileAndOpenDiff(
383+
filePath: string,
384+
originalContent: string,
385+
toolUseId: string,
386+
diffTitle: string
387+
): Promise<{ tempFilePath: string; activeDiffEditor: vscode.TextEditor }> {
388+
const fileName = path.basename(filePath)
389+
const tempFilePath = path.join(path.dirname(filePath), `.amazonq-temp-${toolUseId}-${fileName}`)
390+
const tempFileUri = vscode.Uri.file(tempFilePath)
391+
392+
// Create virtual URI for original content
393+
const originalUri = vscode.Uri.parse(`${diffViewUriScheme}:${fileName}`).with({
394+
query: Buffer.from(originalContent).toString('base64'),
395+
})
396+
397+
// Create temporary file with original content
398+
await this.createTempFile(tempFilePath, originalContent)
399+
400+
// Open diff view between virtual original and temp file
401+
const activeDiffEditor = await new Promise<vscode.TextEditor>((resolve, reject) => {
402+
const disposable = vscode.window.onDidChangeActiveTextEditor((editor) => {
403+
if (editor && editor.document.uri.fsPath === tempFilePath) {
404+
disposable.dispose()
405+
resolve(editor)
406+
}
407+
})
408+
409+
void vscode.commands.executeCommand('vscode.diff', originalUri, tempFileUri, diffTitle, {
410+
preserveFocus: true,
411+
preview: false,
412+
})
413+
414+
// Timeout after 10 seconds
415+
setTimeout(() => {
416+
disposable.dispose()
417+
reject(new Error('Failed to open diff editor within timeout'))
418+
}, 10000)
419+
})
420+
421+
return { tempFilePath, activeDiffEditor }
422+
}
423+
379424
/**
380425
* Configure VSCode's diff editor settings for better focus on changed regions
381426
*/
@@ -528,54 +573,21 @@ export class StreamingDiffController implements vscode.Disposable {
528573
const originalContent = originalDocument.getText()
529574
const fileName = path.basename(fsReplaceComplete.filePath)
530575

531-
// **CRITICAL FIX: Create temporary file for animation (like fsWrite)**
532-
const tempFilePath = path.join(
533-
path.dirname(fsReplaceComplete.filePath),
534-
`.amazonq-temp-${fsReplaceComplete.toolUseId}-${fileName}`
576+
// **STEP 1 & 2: Use helper method to create temp file and open diff view**
577+
const { tempFilePath, activeDiffEditor } = await this.createTempFileAndOpenDiff(
578+
fsReplaceComplete.filePath,
579+
originalContent,
580+
fsReplaceComplete.toolUseId,
581+
`${fileName}: Original ↔ Amazon Q fsReplace Changes`
535582
)
536583
const tempFileUri = vscode.Uri.file(tempFilePath)
537584

538-
// Create virtual URI for original content
539-
const originalUri = vscode.Uri.parse(`${diffViewUriScheme}:${fileName}`).with({
540-
query: Buffer.from(originalContent).toString('base64'),
541-
})
542-
543-
// **STEP 1: Create temp file with original content**
544-
await this.createTempFile(tempFilePath, originalContent)
545-
546-
// **STEP 2: Apply all diffs to create final content**
585+
// **STEP 3: Apply all diffs to create final content**
547586
let finalContent = originalContent
548587
for (const { oldStr, newStr } of structuredDiffs) {
549588
finalContent = finalContent.replace(oldStr, newStr)
550589
}
551590

552-
// **STEP 3: Open diff view between original (virtual) and temp file**
553-
const activeDiffEditor = await new Promise<vscode.TextEditor>((resolve, reject) => {
554-
const disposable = vscode.window.onDidChangeActiveTextEditor((editor) => {
555-
if (editor && editor.document.uri.fsPath === tempFilePath) {
556-
disposable.dispose()
557-
resolve(editor)
558-
}
559-
})
560-
561-
void vscode.commands.executeCommand(
562-
'vscode.diff',
563-
originalUri,
564-
tempFileUri,
565-
`${fileName}: Original ↔ Amazon Q fsReplace Changes`,
566-
{
567-
preserveFocus: true,
568-
preview: false,
569-
}
570-
)
571-
572-
// Timeout after 10 seconds
573-
setTimeout(() => {
574-
disposable.dispose()
575-
reject(new Error('Failed to open diff editor within timeout'))
576-
}, 10000)
577-
})
578-
579591
// **STEP 4: Configure diff editor settings**
580592
await this.configureDiffEditorSettings(activeDiffEditor)
581593

packages/amazonq/src/lsp/chat/messages.ts

Lines changed: 17 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,7 @@ import { decryptResponse, encryptRequest } from '../encryption'
7979
import { focusAmazonQPanel } from './commands'
8080
import { DiffAnimationHandler } from './diffAnimation/diffAnimationHandler'
8181
import { getLogger } from 'aws-core-vscode/shared'
82+
import { getCursorState } from '../utils'
8283

8384
// Create a singleton instance of DiffAnimationHandler
8485
let diffAnimationHandler: DiffAnimationHandler | undefined
@@ -130,21 +131,6 @@ export function registerLanguageServerEventListener(languageClient: LanguageClie
130131
})
131132
}
132133

133-
function getCursorState(selection: readonly vscode.Selection[]) {
134-
return selection.map((s) => ({
135-
range: {
136-
start: {
137-
line: s.start.line,
138-
character: s.start.character,
139-
},
140-
end: {
141-
line: s.end.line,
142-
character: s.end.character,
143-
},
144-
},
145-
}))
146-
}
147-
148134
// Initialize DiffAnimationHandler on first use
149135
function getDiffAnimationHandler(): DiffAnimationHandler {
150136
if (!diffAnimationHandler) {
@@ -153,6 +139,20 @@ function getDiffAnimationHandler(): DiffAnimationHandler {
153139
return diffAnimationHandler
154140
}
155141

142+
// Helper function to clean up temp files - eliminates code duplication
143+
async function cleanupTempFiles(context: string): Promise<void> {
144+
try {
145+
const animationHandler = getDiffAnimationHandler()
146+
const streamingController = (animationHandler as any).streamingDiffController
147+
if (streamingController && streamingController.cleanupChatSession) {
148+
await streamingController.cleanupChatSession()
149+
getLogger().info(`[VSCode Client] 🧹 Cleaned up temp files ${context}`)
150+
}
151+
} catch (error) {
152+
getLogger().warn(`[VSCode Client] ⚠️ Failed to cleanup temp files ${context}: ${error}`)
153+
}
154+
}
155+
156156
export function registerMessageListeners(
157157
languageClient: LanguageClient,
158158
provider: AmazonQChatViewProvider,
@@ -285,16 +285,7 @@ export function registerMessageListeners(
285285

286286
// **CRITICAL FIX**: Clean up temp files when chat is stopped
287287
// This ensures temp files are cleaned up when users stop ongoing operations
288-
try {
289-
const animationHandler = getDiffAnimationHandler()
290-
const streamingController = (animationHandler as any).streamingDiffController
291-
if (streamingController && streamingController.cleanupChatSession) {
292-
await streamingController.cleanupChatSession()
293-
getLogger().info(`[VSCode Client] 🧹 Cleaned up temp files after stopping chat`)
294-
}
295-
} catch (error) {
296-
getLogger().warn(`[VSCode Client] ⚠️ Failed to cleanup temp files after stopping chat: ${error}`)
297-
}
288+
await cleanupTempFiles('after stopping chat')
298289
break
299290
}
300291
case chatRequestType.method: {
@@ -306,16 +297,7 @@ export function registerMessageListeners(
306297

307298
// **CRITICAL FIX**: Clean up temp files from previous chat sessions
308299
// This ensures temp files don't accumulate when users start new conversations
309-
try {
310-
const animationHandler = getDiffAnimationHandler()
311-
const streamingController = (animationHandler as any).streamingDiffController
312-
if (streamingController && streamingController.cleanupChatSession) {
313-
await streamingController.cleanupChatSession()
314-
getLogger().info(`[VSCode Client] 🧹 Cleaned up temp files before starting new chat`)
315-
}
316-
} catch (error) {
317-
getLogger().warn(`[VSCode Client] ⚠️ Failed to cleanup temp files before new chat: ${error}`)
318-
}
300+
await cleanupTempFiles('before starting new chat')
319301

320302
const chatDisposable = languageClient.onProgress(
321303
chatRequestType,

0 commit comments

Comments
 (0)