Skip to content

Commit aefe920

Browse files
authored
fix(chat): use diff lib to get diffs (aws#6940)
## Problem Use https://www.npmjs.com/package/diff for diffs ## Solution - Update diffs for FsWrite - Remove hardcoded changed lines - Remove hardcoded diff language - Use file name instead of file path in card header <img width="754" alt="image" src="https://github.com/user-attachments/assets/aa876778-3fca-48be-99c4-697983f3aabe" /> --- - Treat all work as PUBLIC. Private `feature/x` branches will not be squash-merged at release time. - Your code changes must meet the guidelines in [CONTRIBUTING.md](https://github.com/aws/aws-toolkit-vscode/blob/master/CONTRIBUTING.md#guidelines). - License: I confirm that my contribution is made under the terms of the Apache 2.0 license.
1 parent 122f718 commit aefe920

File tree

5 files changed

+117
-114
lines changed

5 files changed

+117
-114
lines changed

packages/core/src/codewhispererChat/controllers/chat/messenger/messenger.ts

Lines changed: 24 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -44,9 +44,10 @@ import { ChatItemButton, ChatItemContent, ChatItemFormItem, MynahIconsType, Myna
4444
import { ChatHistoryManager } from '../../../storages/chatHistory'
4545
import { ToolType, ToolUtils } from '../../../tools/toolUtils'
4646
import { ChatStream } from '../../../tools/chatStream'
47-
import { getWorkspaceForFile } from '../../../../shared/utilities/workspaceUtils'
4847
import path from 'path'
4948
import { CommandValidation } from '../../../tools/executeBash'
49+
import { Change } from 'diff'
50+
import { FsWriteParams } from '../../../tools/fsWrite'
5051

5152
export type StaticTextResponseType = 'quick-action-help' | 'onboarding-help' | 'transform' | 'help'
5253

@@ -219,8 +220,10 @@ export class Messenger {
219220

220221
const tool = ToolUtils.tryFromToolUse(toolUse)
221222
if ('type' in tool) {
223+
let changeList: Change[] | undefined = undefined
222224
if (tool.type === ToolType.FsWrite) {
223225
session.setShowDiffOnFileWrite(true)
226+
changeList = await tool.tool.getDiffChanges()
224227
}
225228
if (
226229
tool.type === ToolType.FsWrite ||
@@ -239,7 +242,8 @@ export class Messenger {
239242
triggerID,
240243
toolUse,
241244
validation,
242-
session.messageIdToUpdate
245+
session.messageIdToUpdate,
246+
changeList
243247
)
244248
await ToolUtils.queueDescription(tool, chatStream)
245249

@@ -480,7 +484,8 @@ export class Messenger {
480484
triggerID: string,
481485
toolUse: ToolUse | undefined,
482486
validation: CommandValidation,
483-
messageIdToUpdate: string | undefined
487+
messageIdToUpdate: string | undefined,
488+
changeList?: Change[]
484489
) {
485490
const buttons: ChatItemButton[] = []
486491
let fileList: ChatItemContent['fileList'] = undefined
@@ -495,24 +500,29 @@ export class Messenger {
495500
message = validation.warning + message
496501
}
497502
} else if (toolUse?.name === ToolType.FsWrite) {
498-
const absoluteFilePath = (toolUse?.input as any).path
499-
const projectPath = getWorkspaceForFile(absoluteFilePath)
500-
const relativePath = projectPath ? path.relative(projectPath, absoluteFilePath) : absoluteFilePath
503+
const input = toolUse.input as unknown as FsWriteParams
504+
const fileName = path.basename(input.path)
505+
const changes = changeList?.reduce(
506+
(acc, { count = 0, added, removed }) => {
507+
if (added) {
508+
acc.added += count
509+
} else if (removed) {
510+
acc.deleted += count
511+
}
512+
return acc
513+
},
514+
{ added: 0, deleted: 0 }
515+
)
501516
// FileList
502517
fileList = {
503518
fileTreeTitle: '',
504519
hideFileCount: true,
505-
filePaths: [relativePath],
520+
filePaths: [fileName],
506521
details: {
507-
[relativePath]: {
522+
[fileName]: {
508523
// eslint-disable-next-line unicorn/no-null
509524
icon: null,
510-
label: 'Created',
511-
changes: {
512-
added: 36,
513-
deleted: 0,
514-
total: 36,
515-
},
525+
changes: changes,
516526
},
517527
},
518528
}

packages/core/src/codewhispererChat/tools/chatStream.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import { getLogger } from '../../shared/logger/logger'
88
import { Messenger } from '../controllers/chat/messenger/messenger'
99
import { ToolUse } from '@amzn/codewhisperer-streaming'
1010
import { CommandValidation } from './executeBash'
11+
import { Change } from 'diff'
1112

1213
/**
1314
* A writable stream that feeds each chunk/line to the chat UI.
@@ -23,6 +24,7 @@ export class ChatStream extends Writable {
2324
private readonly toolUse: ToolUse | undefined,
2425
private readonly validation: CommandValidation,
2526
private readonly messageIdToUpdate: string | undefined,
27+
private readonly changeList?: Change[],
2628
private readonly logger = getLogger('chatStream')
2729
) {
2830
super()
@@ -44,7 +46,8 @@ export class ChatStream extends Writable {
4446
this.triggerID,
4547
this.toolUse,
4648
this.validation,
47-
this.messageIdToUpdate
49+
this.messageIdToUpdate,
50+
this.changeList
4851
)
4952
callback()
5053
}

packages/core/src/codewhispererChat/tools/fsWrite.ts

Lines changed: 40 additions & 99 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,9 @@ import { getLogger } from '../../shared/logger/logger'
88
import vscode from 'vscode'
99
import { fs } from '../../shared/fs/fs'
1010
import { Writable } from 'stream'
11+
import { Change, diffLines } from 'diff'
12+
import { getDiffMarkdown } from '../../shared/utilities/diffUtils'
13+
import { getLanguageForFilePath } from '../../shared/utilities/textDocumentUtilities'
1114

1215
interface BaseParams {
1316
path: string
@@ -69,114 +72,40 @@ export class FsWrite {
6972
}
7073
}
7174

72-
// TODO: Refactor the fsWrite.ts file "queueDescription" method to use existing diffLines and diff library to get the diff preview. or reuse existing diff view logic in cwchat. This will be part of next PR.
73-
private showStrReplacePreview(oldStr: string, newStr: string): string {
74-
// Split both strings into arrays of lines
75-
const oldStrLines = oldStr.split('\n')
76-
const newStrLines = newStr.split('\n')
77-
let result = ''
78-
79-
// If strings are identical, return empty string
80-
if (oldStr === newStr) {
81-
return result
82-
}
83-
84-
let oldLineIndex = 0
85-
let newLineIndex = 0
86-
// Loop through both arrays until we've processed all lines
87-
while (oldLineIndex < oldStrLines.length || newLineIndex < newStrLines.length) {
88-
if (
89-
oldLineIndex < oldStrLines.length &&
90-
newLineIndex < newStrLines.length &&
91-
oldStrLines[oldLineIndex] === newStrLines[newLineIndex]
92-
) {
93-
// Line is unchanged - prefix with space
94-
result += ` ${oldStrLines[oldLineIndex]}\n`
95-
oldLineIndex++
96-
newLineIndex++
97-
} else {
98-
// Line is different
99-
if (oldLineIndex < oldStrLines.length) {
100-
// Remove line - prefix with minus
101-
result += `- ${oldStrLines[oldLineIndex]}\n`
102-
oldLineIndex++
103-
}
104-
if (newLineIndex < newStrLines.length) {
105-
// Add line - prefix with plus
106-
result += `+ ${newStrLines[newLineIndex]}\n`
107-
newLineIndex++
108-
}
109-
}
110-
}
111-
112-
return result
113-
}
114-
115-
private async showInsertPreview(path: string, insertLine: number, newStr: string): Promise<string> {
116-
const fileContent = await fs.readFileText(path)
117-
const lines = fileContent.split('\n')
118-
const startLine = Math.max(0, insertLine - 2)
119-
const endLine = Math.min(lines.length, insertLine + 3)
120-
121-
const contextLines: string[] = []
122-
123-
// Add lines before insertion point
124-
for (let index = startLine; index < insertLine; index++) {
125-
contextLines.push(` ${lines[index]}`)
126-
}
127-
128-
// Add the new line with a '+' prefix
129-
contextLines.push(`+ ${newStr}`)
130-
131-
// Add lines after insertion point
132-
for (let index = insertLine; index < endLine; index++) {
133-
contextLines.push(` ${lines[index]}`)
134-
}
75+
public async queueDescription(updates: Writable): Promise<void> {
76+
const sanitizedPath = sanitizePath(this.params.path)
77+
const changes = await this.getDiffChanges()
78+
const language = await getLanguageForFilePath(sanitizedPath)
13579

136-
return contextLines.join('\n')
80+
const diff = getDiffMarkdown(changes, language)
81+
updates.write(diff)
82+
updates.end()
13783
}
13884

139-
private async showAppendPreview(sanitizedPath: string, newStr: string) {
140-
const fileContent = await fs.readFileText(sanitizedPath)
141-
const needsNewline = fileContent.length !== 0 && !fileContent.endsWith('\n')
142-
143-
let contentToAppend = newStr
144-
if (needsNewline) {
145-
contentToAppend = '\n' + contentToAppend
85+
public async getDiffChanges(): Promise<Change[]> {
86+
const sanitizedPath = sanitizePath(this.params.path)
87+
let newContent
88+
let oldContent
89+
try {
90+
oldContent = await fs.readFileText(sanitizedPath)
91+
} catch (err) {
92+
oldContent = ''
14693
}
147-
148-
// Get the last 3 lines from existing content for better UX
149-
const lines = fileContent.split('\n')
150-
const linesForContext = lines.slice(-3)
151-
152-
return `${linesForContext.join('\n')}\n+ ${contentToAppend.trim()}`
153-
}
154-
155-
public async queueDescription(updates: Writable): Promise<void> {
15694
switch (this.params.command) {
15795
case 'create':
158-
updates.write(`\`\`\`diff-typescript
159-
${'+' + this.params.fileText?.replace(/\n/g, '\n+')}
160-
`)
96+
newContent = this.getCreateCommandText(this.params)
16197
break
16298
case 'strReplace':
163-
updates.write(`\`\`\`diff-typescript
164-
${this.showStrReplacePreview(this.params.oldStr, this.params.newStr)}
165-
\`\`\`
166-
`)
99+
newContent = await this.getStrReplaceContent(this.params, sanitizedPath)
167100
break
168101
case 'insert':
169-
updates.write(`\`\`\`diff-typescript
170-
${await this.showInsertPreview(this.params.path, this.params.insertLine, this.params.newStr)}
171-
\`\`\``)
102+
newContent = await this.getInsertContent(this.params, sanitizedPath)
172103
break
173104
case 'append':
174-
updates.write(`\`\`\`diff-typescript
175-
${await this.showAppendPreview(this.params.path, this.params.newStr)}
176-
\`\`\``)
105+
newContent = await this.getAppendContent(this.params, sanitizedPath)
177106
break
178107
}
179-
updates.end()
108+
return diffLines(oldContent, newContent)
180109
}
181110

182111
public async validate(): Promise<void> {
@@ -224,6 +153,11 @@ ${await this.showAppendPreview(this.params.path, this.params.newStr)}
224153
}
225154

226155
private async handleStrReplace(params: StrReplaceParams, sanitizedPath: string): Promise<void> {
156+
const newContent = await this.getStrReplaceContent(params, sanitizedPath)
157+
await fs.writeFile(sanitizedPath, newContent)
158+
}
159+
160+
private async getStrReplaceContent(params: StrReplaceParams, sanitizedPath: string): Promise<string> {
227161
const fileContent = await fs.readFileText(sanitizedPath)
228162

229163
const matches = [...fileContent.matchAll(new RegExp(this.escapeRegExp(params.oldStr), 'g'))]
@@ -235,11 +169,15 @@ ${await this.showAppendPreview(this.params.path, this.params.newStr)}
235169
throw new Error(`${matches.length} occurrences of oldStr were found when only 1 is expected`)
236170
}
237171

238-
const newContent = fileContent.replace(params.oldStr, params.newStr)
239-
await fs.writeFile(sanitizedPath, newContent)
172+
return fileContent.replace(params.oldStr, params.newStr)
240173
}
241174

242175
private async handleInsert(params: InsertParams, sanitizedPath: string): Promise<void> {
176+
const newContent = await this.getInsertContent(params, sanitizedPath)
177+
await fs.writeFile(sanitizedPath, newContent)
178+
}
179+
180+
private async getInsertContent(params: InsertParams, sanitizedPath: string): Promise<string> {
243181
const fileContent = await fs.readFileText(sanitizedPath)
244182
const lines = fileContent.split('\n')
245183

@@ -252,11 +190,15 @@ ${await this.showAppendPreview(this.params.path, this.params.newStr)}
252190
} else {
253191
newContent = [...lines.slice(0, insertLine), params.newStr, ...lines.slice(insertLine)].join('\n')
254192
}
193+
return newContent
194+
}
255195

196+
private async handleAppend(params: AppendParams, sanitizedPath: string): Promise<void> {
197+
const newContent = await this.getAppendContent(params, sanitizedPath)
256198
await fs.writeFile(sanitizedPath, newContent)
257199
}
258200

259-
private async handleAppend(params: AppendParams, sanitizedPath: string): Promise<void> {
201+
private async getAppendContent(params: AppendParams, sanitizedPath: string): Promise<string> {
260202
const fileContent = await fs.readFileText(sanitizedPath)
261203
const needsNewline = fileContent.length !== 0 && !fileContent.endsWith('\n')
262204

@@ -265,8 +207,7 @@ ${await this.showAppendPreview(this.params.path, this.params.newStr)}
265207
contentToAppend = '\n' + contentToAppend
266208
}
267209

268-
const newContent = fileContent + contentToAppend
269-
await fs.writeFile(sanitizedPath, newContent)
210+
return fileContent + contentToAppend
270211
}
271212

272213
private getCreateCommandText(params: CreateParams): string {

packages/core/src/shared/utilities/diffUtils.ts

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -149,3 +149,31 @@ export function getDiffCharsAndLines(
149149
addedLines,
150150
}
151151
}
152+
153+
/**
154+
* Converts diff changes into a markdown-formatted string with syntax highlighting.
155+
* This function takes an array of diff changes and formats them as a markdown code block
156+
* with diff syntax, optionally including language-specific syntax highlighting.
157+
*
158+
* @param {Change[]} changes - An array of diff Change objects containing added, removed, or unchanged content
159+
* @param {string} [language] - Optional language identifier for syntax highlighting in the markdown output
160+
* @returns {string} A markdown-formatted string representing the diff with proper syntax highlighting
161+
* @example
162+
* // Generate diff markdown for JavaScript changes
163+
* const diffChanges = diffLines(originalCode, newCode);
164+
* const markdown = getDiffMarkdown(diffChanges, 'javascript');
165+
* // Result will be a markdown code block with diff syntax and JavaScript highlighting
166+
*/
167+
export function getDiffMarkdown(changes: Change[], language?: string): string {
168+
return ['```diff' + (language ? `-${language}` : ''), ...changes.flatMap(formatDiffPart), '```'].join('\n')
169+
}
170+
171+
function formatDiffPart(part: Change) {
172+
const prefix = part.added ? '+' : part.removed ? '-' : ' '
173+
const lines = part.value.split('\n')
174+
175+
if (lines.length > 0 && lines[lines.length - 1] === '') {
176+
lines.pop()
177+
}
178+
return lines.map((line) => `${prefix}${line}`)
179+
}

packages/core/src/shared/utilities/textDocumentUtilities.ts

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -224,6 +224,27 @@ export async function addEofNewline(editor: vscode.TextEditor) {
224224
}
225225
}
226226

227+
/**
228+
* Determines the language identifier for a file at the specified path.
229+
* This function attempts to open the file as a VS Code document to retrieve its language ID.
230+
*
231+
* @param {string} filePath - The absolute or relative path to the file whose language should be determined.
232+
* @returns {Promise<string | undefined>} A promise that resolves to the language identifier of the file,
233+
* or undefined if the file cannot be opened or the language cannot be determined.
234+
* @example
235+
* // Get language for a JavaScript file
236+
* const language = await getLanguageForFilePath('/path/to/file.js');
237+
* // language will be 'javascript'
238+
*/
239+
export async function getLanguageForFilePath(filePath: string): Promise<string | undefined> {
240+
try {
241+
const document = await vscode.workspace.openTextDocument(filePath)
242+
return document.languageId
243+
} catch (err) {
244+
return
245+
}
246+
}
247+
227248
class ReadonlyTextDocumentProvider implements vscode.TextDocumentContentProvider {
228249
private content = ''
229250

0 commit comments

Comments
 (0)