Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -44,9 +44,10 @@ import { ChatItemButton, ChatItemContent, ChatItemFormItem, MynahIconsType, Myna
import { ChatHistoryManager } from '../../../storages/chatHistory'
import { ToolType, ToolUtils } from '../../../tools/toolUtils'
import { ChatStream } from '../../../tools/chatStream'
import { getWorkspaceForFile } from '../../../../shared/utilities/workspaceUtils'
import path from 'path'
import { CommandValidation } from '../../../tools/executeBash'
import { Change } from 'diff'
import { FsWriteParams } from '../../../tools/fsWrite'

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

Expand Down Expand Up @@ -219,8 +220,10 @@ export class Messenger {

const tool = ToolUtils.tryFromToolUse(toolUse)
if ('type' in tool) {
let changeList: Change[] | undefined = undefined
if (tool.type === ToolType.FsWrite) {
session.setShowDiffOnFileWrite(true)
changeList = await tool.tool.getDiffChanges()
}
if (
tool.type === ToolType.FsWrite ||
Expand All @@ -239,7 +242,8 @@ export class Messenger {
triggerID,
toolUse,
validation,
session.messageIdToUpdate
session.messageIdToUpdate,
changeList
)
await ToolUtils.queueDescription(tool, chatStream)

Expand Down Expand Up @@ -480,7 +484,8 @@ export class Messenger {
triggerID: string,
toolUse: ToolUse | undefined,
validation: CommandValidation,
messageIdToUpdate: string | undefined
messageIdToUpdate: string | undefined,
changeList?: Change[]
) {
const buttons: ChatItemButton[] = []
let fileList: ChatItemContent['fileList'] = undefined
Expand All @@ -495,24 +500,29 @@ export class Messenger {
message = validation.warning + message
}
} else if (toolUse?.name === ToolType.FsWrite) {
const absoluteFilePath = (toolUse?.input as any).path
const projectPath = getWorkspaceForFile(absoluteFilePath)
const relativePath = projectPath ? path.relative(projectPath, absoluteFilePath) : absoluteFilePath
const input = toolUse.input as unknown as FsWriteParams
const fileName = path.basename(input.path)
const changes = changeList?.reduce(
(acc, { count = 0, added, removed }) => {
if (added) {
acc.added += count
} else if (removed) {
acc.deleted += count
}
return acc
},
{ added: 0, deleted: 0 }
)
// FileList
fileList = {
fileTreeTitle: '',
hideFileCount: true,
filePaths: [relativePath],
filePaths: [fileName],
details: {
[relativePath]: {
[fileName]: {
// eslint-disable-next-line unicorn/no-null
icon: null,
label: 'Created',
changes: {
added: 36,
deleted: 0,
total: 36,
},
changes: changes,
},
},
}
Expand Down
5 changes: 4 additions & 1 deletion packages/core/src/codewhispererChat/tools/chatStream.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import { getLogger } from '../../shared/logger/logger'
import { Messenger } from '../controllers/chat/messenger/messenger'
import { ToolUse } from '@amzn/codewhisperer-streaming'
import { CommandValidation } from './executeBash'
import { Change } from 'diff'

/**
* A writable stream that feeds each chunk/line to the chat UI.
Expand All @@ -23,6 +24,7 @@ export class ChatStream extends Writable {
private readonly toolUse: ToolUse | undefined,
private readonly validation: CommandValidation,
private readonly messageIdToUpdate: string | undefined,
private readonly changeList?: Change[],
private readonly logger = getLogger('chatStream')
) {
super()
Expand All @@ -44,7 +46,8 @@ export class ChatStream extends Writable {
this.triggerID,
this.toolUse,
this.validation,
this.messageIdToUpdate
this.messageIdToUpdate,
this.changeList
)
callback()
}
Expand Down
139 changes: 40 additions & 99 deletions packages/core/src/codewhispererChat/tools/fsWrite.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@ import { getLogger } from '../../shared/logger/logger'
import vscode from 'vscode'
import { fs } from '../../shared/fs/fs'
import { Writable } from 'stream'
import { Change, diffLines } from 'diff'
import { getDiffMarkdown } from '../../shared/utilities/diffUtils'
import { getLanguageForFilePath } from '../../shared/utilities/textDocumentUtilities'

interface BaseParams {
path: string
Expand Down Expand Up @@ -69,114 +72,40 @@ export class FsWrite {
}
}

// 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.
private showStrReplacePreview(oldStr: string, newStr: string): string {
// Split both strings into arrays of lines
const oldStrLines = oldStr.split('\n')
const newStrLines = newStr.split('\n')
let result = ''

// If strings are identical, return empty string
if (oldStr === newStr) {
return result
}

let oldLineIndex = 0
let newLineIndex = 0
// Loop through both arrays until we've processed all lines
while (oldLineIndex < oldStrLines.length || newLineIndex < newStrLines.length) {
if (
oldLineIndex < oldStrLines.length &&
newLineIndex < newStrLines.length &&
oldStrLines[oldLineIndex] === newStrLines[newLineIndex]
) {
// Line is unchanged - prefix with space
result += ` ${oldStrLines[oldLineIndex]}\n`
oldLineIndex++
newLineIndex++
} else {
// Line is different
if (oldLineIndex < oldStrLines.length) {
// Remove line - prefix with minus
result += `- ${oldStrLines[oldLineIndex]}\n`
oldLineIndex++
}
if (newLineIndex < newStrLines.length) {
// Add line - prefix with plus
result += `+ ${newStrLines[newLineIndex]}\n`
newLineIndex++
}
}
}

return result
}

private async showInsertPreview(path: string, insertLine: number, newStr: string): Promise<string> {
const fileContent = await fs.readFileText(path)
const lines = fileContent.split('\n')
const startLine = Math.max(0, insertLine - 2)
const endLine = Math.min(lines.length, insertLine + 3)

const contextLines: string[] = []

// Add lines before insertion point
for (let index = startLine; index < insertLine; index++) {
contextLines.push(` ${lines[index]}`)
}

// Add the new line with a '+' prefix
contextLines.push(`+ ${newStr}`)

// Add lines after insertion point
for (let index = insertLine; index < endLine; index++) {
contextLines.push(` ${lines[index]}`)
}
public async queueDescription(updates: Writable): Promise<void> {
const sanitizedPath = sanitizePath(this.params.path)
const changes = await this.getDiffChanges()
const language = await getLanguageForFilePath(sanitizedPath)

return contextLines.join('\n')
const diff = getDiffMarkdown(changes, language)
updates.write(diff)
updates.end()
}

private async showAppendPreview(sanitizedPath: string, newStr: string) {
const fileContent = await fs.readFileText(sanitizedPath)
const needsNewline = fileContent.length !== 0 && !fileContent.endsWith('\n')

let contentToAppend = newStr
if (needsNewline) {
contentToAppend = '\n' + contentToAppend
public async getDiffChanges(): Promise<Change[]> {
const sanitizedPath = sanitizePath(this.params.path)
let newContent
let oldContent
try {
oldContent = await fs.readFileText(sanitizedPath)
} catch (err) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we log it? what is the expected UX if we cannot read the file, do we still want to proceed with showing diffs?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LLM sends the filePath which may or may not exist
If fileExists we take the oldContent if not we consider this as a new file and keep oldContent as ''.
I think we should not inform end customer about this!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah in the create command case, this is expected to fail because the file doesn't exist yet. So I don't think its necessary to show an error

oldContent = ''
}

// Get the last 3 lines from existing content for better UX
const lines = fileContent.split('\n')
const linesForContext = lines.slice(-3)

return `${linesForContext.join('\n')}\n+ ${contentToAppend.trim()}`
}

public async queueDescription(updates: Writable): Promise<void> {
switch (this.params.command) {
case 'create':
updates.write(`\`\`\`diff-typescript
${'+' + this.params.fileText?.replace(/\n/g, '\n+')}
`)
newContent = this.getCreateCommandText(this.params)
break
case 'strReplace':
updates.write(`\`\`\`diff-typescript
${this.showStrReplacePreview(this.params.oldStr, this.params.newStr)}
\`\`\`
`)
newContent = await this.getStrReplaceContent(this.params, sanitizedPath)
break
case 'insert':
updates.write(`\`\`\`diff-typescript
${await this.showInsertPreview(this.params.path, this.params.insertLine, this.params.newStr)}
\`\`\``)
newContent = await this.getInsertContent(this.params, sanitizedPath)
break
case 'append':
updates.write(`\`\`\`diff-typescript
${await this.showAppendPreview(this.params.path, this.params.newStr)}
\`\`\``)
newContent = await this.getAppendContent(this.params, sanitizedPath)
break
}
updates.end()
return diffLines(oldContent, newContent)
}

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

private async handleStrReplace(params: StrReplaceParams, sanitizedPath: string): Promise<void> {
const newContent = await this.getStrReplaceContent(params, sanitizedPath)
await fs.writeFile(sanitizedPath, newContent)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does caller has error handling for these?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes any error thrown from a tool will get sent back to the LLM

}

private async getStrReplaceContent(params: StrReplaceParams, sanitizedPath: string): Promise<string> {
const fileContent = await fs.readFileText(sanitizedPath)

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

const newContent = fileContent.replace(params.oldStr, params.newStr)
await fs.writeFile(sanitizedPath, newContent)
return fileContent.replace(params.oldStr, params.newStr)
}

private async handleInsert(params: InsertParams, sanitizedPath: string): Promise<void> {
const newContent = await this.getInsertContent(params, sanitizedPath)
await fs.writeFile(sanitizedPath, newContent)
}

private async getInsertContent(params: InsertParams, sanitizedPath: string): Promise<string> {
const fileContent = await fs.readFileText(sanitizedPath)
const lines = fileContent.split('\n')

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

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

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

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

const newContent = fileContent + contentToAppend
await fs.writeFile(sanitizedPath, newContent)
return fileContent + contentToAppend
}

private getCreateCommandText(params: CreateParams): string {
Expand Down
28 changes: 28 additions & 0 deletions packages/core/src/shared/utilities/diffUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -149,3 +149,31 @@ export function getDiffCharsAndLines(
addedLines,
}
}

/**
* Converts diff changes into a markdown-formatted string with syntax highlighting.
* This function takes an array of diff changes and formats them as a markdown code block
* with diff syntax, optionally including language-specific syntax highlighting.
*
* @param {Change[]} changes - An array of diff Change objects containing added, removed, or unchanged content
* @param {string} [language] - Optional language identifier for syntax highlighting in the markdown output
* @returns {string} A markdown-formatted string representing the diff with proper syntax highlighting
* @example
* // Generate diff markdown for JavaScript changes
* const diffChanges = diffLines(originalCode, newCode);
* const markdown = getDiffMarkdown(diffChanges, 'javascript');
* // Result will be a markdown code block with diff syntax and JavaScript highlighting
*/
export function getDiffMarkdown(changes: Change[], language?: string): string {
return ['```diff' + (language ? `-${language}` : ''), ...changes.flatMap(formatDiffPart), '```'].join('\n')
}

function formatDiffPart(part: Change) {
const prefix = part.added ? '+' : part.removed ? '-' : ' '
const lines = part.value.split('\n')

if (lines.length > 0 && lines[lines.length - 1] === '') {
lines.pop()
}
return lines.map((line) => `${prefix}${line}`)
}
21 changes: 21 additions & 0 deletions packages/core/src/shared/utilities/textDocumentUtilities.ts
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,27 @@ export async function addEofNewline(editor: vscode.TextEditor) {
}
}

/**
* Determines the language identifier for a file at the specified path.
* This function attempts to open the file as a VS Code document to retrieve its language ID.
*
* @param {string} filePath - The absolute or relative path to the file whose language should be determined.
* @returns {Promise<string | undefined>} A promise that resolves to the language identifier of the file,
* or undefined if the file cannot be opened or the language cannot be determined.
* @example
* // Get language for a JavaScript file
* const language = await getLanguageForFilePath('/path/to/file.js');
* // language will be 'javascript'
*/
export async function getLanguageForFilePath(filePath: string): Promise<string | undefined> {
try {
const document = await vscode.workspace.openTextDocument(filePath)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does this one recognize the languages that's not supported by VS Code natively?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think right now no, the language will have to be recognized by vscode

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

did product ack this? or we can switch to use the file extension matching logic that we have

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will only recognize languages natively supported by Visual Studio Code, though it covers most programming languages. For unsupported languages, it defaults to plaintext.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand that, my question is if this is what product/UX expected, because some of them are technically supported by Q although not by VS Code

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think if we use our file extension matching logic it might be too restrictive? For new languages we have to always update the map.

Another reason I was thinking to use VSCode API is that the markdown diff view and the native diff view should have consistent syntax highlighting

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is ok to follow up

return document.languageId
} catch (err) {
return
}
}

class ReadonlyTextDocumentProvider implements vscode.TextDocumentContentProvider {
private content = ''

Expand Down
Loading