fix(chat): use diff lib to get diffs #6940
Conversation
|
| let oldContent | ||
| try { | ||
| oldContent = await fs.readFileText(sanitizedPath) | ||
| } catch (err) { |
There was a problem hiding this comment.
should we log it? what is the expected UX if we cannot read the file, do we still want to proceed with showing diffs?
There was a problem hiding this comment.
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!
There was a problem hiding this comment.
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
|
|
||
| private async handleStrReplace(params: StrReplaceParams, sanitizedPath: string): Promise<void> { | ||
| const newContent = await this.getStrReplaceContent(params, sanitizedPath) | ||
| await fs.writeFile(sanitizedPath, newContent) |
There was a problem hiding this comment.
does caller has error handling for these?
There was a problem hiding this comment.
yes any error thrown from a tool will get sent back to the LLM
| */ | ||
| export async function getLanguageForFilePath(filePath: string): Promise<string | undefined> { | ||
| try { | ||
| const document = await vscode.workspace.openTextDocument(filePath) |
There was a problem hiding this comment.
does this one recognize the languages that's not supported by VS Code natively?
There was a problem hiding this comment.
I think right now no, the language will have to be recognized by vscode
There was a problem hiding this comment.
did product ack this? or we can switch to use the file extension matching logic that we have
There was a problem hiding this comment.
This will only recognize languages natively supported by Visual Studio Code, though it covers most programming languages. For unsupported languages, it defaults to plaintext.
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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
3 participants
Problem
Use https://www.npmjs.com/package/diff for diffs
Solution
feature/xbranches will not be squash-merged at release time.