Skip to content

Commit af4aa0f

Browse files
justschenjustschen
authored
debt: more image qol and cleanup (#245637)
* more qol on image attachments * some renaming * more cleanup
1 parent 36ea9e0 commit af4aa0f

13 files changed

Lines changed: 114 additions & 61 deletions

File tree

‎src/vs/workbench/api/common/extHostTypeConverters.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ import { DEFAULT_EDITOR_ASSOCIATION, SaveReason } from '../../common/editor.js';
4141
import { IViewBadge } from '../../common/views.js';
4242
import { IChatAgentRequest, IChatAgentResult } from '../../contrib/chat/common/chatAgents.js';
4343
import { IChatRequestDraft } from '../../contrib/chat/common/chatEditingService.js';
44-
import { IChatRequestVariableEntry } from '../../contrib/chat/common/chatModel.js';
44+
import { IChatRequestVariableEntry, isImageVariableEntry } from '../../contrib/chat/common/chatModel.js';
4545
import { IChatAgentMarkdownContentWithVulnerability, IChatCodeCitation, IChatCommandButton, IChatConfirmation, IChatContentInlineReference, IChatContentReference, IChatFollowup, IChatMarkdownContent, IChatMoveMessage, IChatProgressMessage, IChatResponseCodeblockUriPart, IChatTaskDto, IChatTaskResult, IChatTextEdit, IChatTreeData, IChatUserActionEvent, IChatWarningMessage } from '../../contrib/chat/common/chatService.js';
4646
import { IToolData, IToolResult } from '../../contrib/chat/common/languageModelToolsService.js';
4747
import * as chatProvider from '../../contrib/chat/common/languageModels.js';
@@ -2950,7 +2950,7 @@ export namespace ChatPromptReference {
29502950
value = URI.revive(value);
29512951
} else if (value && typeof value === 'object' && 'uri' in value && 'range' in value && isUriComponents(value.uri)) {
29522952
value = Location.to(revive(value));
2953-
} else if (variable.isImage) {
2953+
} else if (isImageVariableEntry(variable)) {
29542954
const ref = variable.references?.[0]?.reference;
29552955
value = new types.ChatReferenceBinaryData(
29562956
variable.mimeType ?? 'image/png',

‎src/vs/workbench/contrib/chat/browser/actions/chatContextActions.ts‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ import { SearchContext } from '../../../search/common/constants.js';
5252
import { IChatAgentService } from '../../common/chatAgents.js';
5353
import { ChatContextKeyExprs, ChatContextKeys } from '../../common/chatContextKeys.js';
5454
import { IChatEditingService } from '../../common/chatEditingService.js';
55-
import { IChatRequestVariableEntry, IDiagnosticVariableEntryFilterData } from '../../common/chatModel.js';
55+
import { IChatRequestVariableEntry, IDiagnosticVariableEntryFilterData, OmittedState } from '../../common/chatModel.js';
5656
import { ChatRequestAgentPart } from '../../common/chatParserTypes.js';
5757
import { IChatVariablesService } from '../../common/chatVariables.js';
5858
import { ChatAgentLocation } from '../../common/constants.js';
@@ -632,24 +632,24 @@ export class AttachContextAction extends Action2 {
632632
name: pick.label,
633633
fullName: pick.label,
634634
value: resizedImage,
635-
isImage: true
635+
kind: 'image',
636636
});
637637
}
638638
} else {
639-
let isOmitted = false;
639+
let omittedState = OmittedState.NotOmitted;
640640
try {
641641
const createdModel = await textModelService.createModelReference(pick.resource);
642642
createdModel.dispose();
643643
} catch {
644-
isOmitted = true;
644+
omittedState = OmittedState.Full;
645645
}
646646

647647
toAttach.push({
648648
id: this._getFileContextId({ resource: pick.resource }),
649649
value: pick.resource,
650650
name: pick.label,
651651
isFile: true,
652-
isOmitted
652+
omittedState
653653
});
654654
}
655655
} else if (isIGotoSymbolQuickPickItem(pick) && pick.uri && pick.range) {
@@ -715,7 +715,7 @@ export class AttachContextAction extends Action2 {
715715
value: file.value,
716716
name: file.label,
717717
isFile: true,
718-
isOmitted: false
718+
omittedState: OmittedState.NotOmitted
719719
});
720720
}
721721
} else if (isScreenshotQuickPickItem(pick)) {
@@ -759,7 +759,7 @@ export class AttachContextAction extends Action2 {
759759
name: localize('pastedImage', 'Pasted Image'),
760760
fullName: localize('pastedImage', 'Pasted Image'),
761761
value: fileBuffer,
762-
isImage: true
762+
kind: 'image',
763763
});
764764
}
765765
}

‎src/vs/workbench/contrib/chat/browser/chatAttachmentModel.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,7 @@ export class ChatAttachmentModel extends Disposable {
114114
name: fileName,
115115
fullName: uri.path,
116116
value: resizedImage,
117-
isImage: true,
117+
kind: 'image',
118118
isFile: false,
119119
references: [{ reference: uri, kind: 'reference' }]
120120
};

‎src/vs/workbench/contrib/chat/browser/chatAttachmentResolve.ts‎

Lines changed: 30 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ import { EditorInput } from '../../../common/editor/editorInput.js';
2020
import { IEditorService } from '../../../services/editor/common/editorService.js';
2121
import { IExtensionService, isProposedApiEnabled } from '../../../services/extensions/common/extensions.js';
2222
import { UntitledTextEditorInput } from '../../../services/untitled/common/untitledTextEditorInput.js';
23-
import { IChatRequestVariableEntry, IDiagnosticVariableEntry, IDiagnosticVariableEntryFilterData, ISymbolVariableEntry } from '../common/chatModel.js';
23+
import { IChatRequestVariableEntry, IDiagnosticVariableEntry, IDiagnosticVariableEntryFilterData, ISymbolVariableEntry, OmittedState } from '../common/chatModel.js';
2424
import { imageToHash } from './chatPasteProviders.js';
2525
import { resizeImage } from './imageUtils.js';
2626

@@ -75,18 +75,18 @@ async function resolveUntitledEditorAttachContext(editor: IDraggedResourceEditor
7575
}
7676

7777
export async function resolveResourceAttachContext(resource: URI, isDirectory: boolean, textModelService: ITextModelService): Promise<IChatRequestVariableEntry | undefined> {
78-
let isOmitted = false;
78+
let omittedState = OmittedState.NotOmitted;
7979

8080
if (!isDirectory) {
8181
try {
8282
const createdModel = await textModelService.createModelReference(resource);
8383
createdModel.dispose();
8484
} catch {
85-
isOmitted = true;
85+
omittedState = OmittedState.Full;
8686
}
8787

8888
if (/\.(svg)$/i.test(resource.path)) {
89-
isOmitted = true;
89+
omittedState = OmittedState.Full;
9090
}
9191
}
9292

@@ -96,7 +96,7 @@ export async function resolveResourceAttachContext(resource: URI, isDirectory: b
9696
name: basename(resource),
9797
isFile: !isDirectory,
9898
isDirectory,
99-
isOmitted
99+
omittedState
100100
};
101101
}
102102

@@ -108,6 +108,8 @@ export type ImageTransferData = {
108108
icon?: ThemeIcon;
109109
resource?: URI;
110110
id?: string;
111+
mimeType?: string;
112+
omittedState?: OmittedState;
111113
};
112114
const SUPPORTED_IMAGE_EXTENSIONS_REGEX = /\.(png|jpg|jpeg|gif|webp)$/i;
113115

@@ -116,23 +118,29 @@ export async function resolveImageEditorAttachContext(editor: EditorInput | IDra
116118
return undefined;
117119
}
118120

119-
if (!SUPPORTED_IMAGE_EXTENSIONS_REGEX.test(editor.resource.path)) {
121+
const match = SUPPORTED_IMAGE_EXTENSIONS_REGEX.exec(editor.resource.path);
122+
if (!match) {
120123
return undefined;
121124
}
122125

126+
const mimeType = getMimeTypeFromPath(match);
123127
const fileName = basename(editor.resource);
124128
const readFile = await fileService.readFile(editor.resource);
129+
125130
if (readFile.size > 30 * 1024 * 1024) { // 30 MB
126131
dialogService.error(localize('imageTooLarge', 'Image is too large'), localize('imageTooLargeMessage', 'The image {0} is too large to be attached.', fileName));
127132
throw new Error('Image is too large');
128133
}
129134

135+
const isPartiallyOmitted = /\.gif$/i.test(editor.resource.path);
130136
const imageFileContext = await resolveImageAttachContext([{
131137
id: editor.resource.toString(),
132138
name: fileName,
133139
data: readFile.value.buffer,
134140
icon: Codicon.fileMedia,
135141
resource: editor.resource,
142+
mimeType: mimeType,
143+
omittedState: isPartiallyOmitted ? OmittedState.Partial : OmittedState.NotOmitted
136144
}]);
137145

138146
return imageFileContext[0];
@@ -143,15 +151,29 @@ export async function resolveImageAttachContext(images: ImageTransferData[]): Pr
143151
id: image.id || await imageToHash(image.data),
144152
name: image.name,
145153
fullName: image.resource ? image.resource.path : undefined,
146-
value: await resizeImage(image.data),
154+
value: await resizeImage(image.data, image.mimeType),
147155
icon: image.icon,
148-
isImage: true,
156+
kind: 'image',
149157
isFile: false,
150158
isDirectory: false,
159+
omittedState: image.omittedState || OmittedState.NotOmitted,
151160
references: image.resource ? [{ reference: image.resource, kind: 'reference' }] : []
152161
})));
153162
}
154163

164+
const MIME_TYPES: Record<string, string> = {
165+
png: 'image/png',
166+
jpg: 'image/jpeg',
167+
jpeg: 'image/jpeg',
168+
gif: 'image/gif',
169+
webp: 'image/webp',
170+
};
171+
172+
function getMimeTypeFromPath(match: RegExpExecArray): string | undefined {
173+
const ext = match[1].toLowerCase();
174+
return MIME_TYPES[ext];
175+
}
176+
155177
// --- MARKERS ---
156178

157179
export function resolveMarkerAttachContext(markers: MarkerTransferData[]): IDiagnosticVariableEntry[] {

‎src/vs/workbench/contrib/chat/browser/chatAttachmentWidgets.ts‎

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ import { IOpenerService, OpenInternalOptions } from '../../../../platform/opener
2525
import { IThemeService, FolderThemeIcon } from '../../../../platform/theme/common/themeService.js';
2626
import { IResourceLabel, ResourceLabels, IFileLabelOptions } from '../../../browser/labels.js';
2727
import { revealInSideBarCommand } from '../../files/browser/fileActions.contribution.js';
28-
import { IChatRequestPasteVariableEntry, IChatRequestVariableEntry } from '../common/chatModel.js';
28+
import { IChatRequestPasteVariableEntry, IChatRequestVariableEntry, OmittedState } from '../common/chatModel.js';
2929
import { ILanguageModelChatMetadataAndIdentifier, ILanguageModelsService } from '../common/languageModels.js';
3030
import { hookUpResourceAttachmentDragAndContextMenu, hookUpSymbolAttachmentDragAndContextMenu } from './chatContentParts/chatAttachmentsContentPart.js';
3131
import { KeyCode } from '../../../../base/common/keyCodes.js';
@@ -149,7 +149,7 @@ export class FileAttachmentWidget extends AbstractChatAttachmentWidget {
149149
const ariaLabel = range ? localize('chat.fileAttachmentWithRange', "Attached file, {0}, line {1} to line {2}", friendlyName, range.startLineNumber, range.endLineNumber) : localize('chat.fileAttachment', "Attached file, {0}", friendlyName);
150150
this.element.ariaLabel = ariaLabel;
151151

152-
if (attachment.isOmitted) {
152+
if (attachment.omittedState === OmittedState.Full) {
153153
this.renderOmittedWarning(friendlyName, ariaLabel, hoverDelegate);
154154
} else {
155155
const fileOptions: IFileLabelOptions = { hidePath: true };
@@ -206,7 +206,15 @@ export class ImageAttachmentWidget extends AbstractChatAttachmentWidget {
206206
) {
207207
super(attachment, shouldFocusClearButton, container, contextResourceLabels, hoverDelegate, currentLanguageModel, commandService, openerService);
208208

209-
const ariaLabel = localize('chat.imageAttachment', "Attached image, {0}", attachment.name);
209+
let ariaLabel: string;
210+
if (attachment.omittedState === OmittedState.Full) {
211+
ariaLabel = localize('chat.omittedImageAttachment', "Omitted this image: {0}", attachment.name);
212+
} else if (attachment.omittedState === OmittedState.Partial) {
213+
ariaLabel = localize('chat.partiallyOmittedImageAttachment', "Partially omitted this image: {0}", attachment.name);
214+
} else {
215+
ariaLabel = localize('chat.imageAttachment', "Attached image, {0}", attachment.name);
216+
}
217+
210218
this.element.ariaLabel = ariaLabel;
211219
this.element.style.position = 'relative';
212220

@@ -252,7 +260,7 @@ export class ImageAttachmentWidget extends AbstractChatAttachmentWidget {
252260
this._register(this.hoverService.setupDelayedHover(this.element, { content: hoverElement, appearance: { showPointer: true } }));
253261
} else {
254262
const buffer = attachment.value as Uint8Array;
255-
this.createImageElements(buffer, this.element, hoverElement, URI.isUri(ref) ? ref : undefined);
263+
this.createImageElements(buffer, this.element, hoverElement, URI.isUri(ref) ? ref : undefined, attachment.omittedState);
256264
this._register(this.hoverService.setupDelayedHover(this.element, { content: hoverElement, appearance: { showPointer: true } }));
257265
}
258266

@@ -263,7 +271,11 @@ export class ImageAttachmentWidget extends AbstractChatAttachmentWidget {
263271
this.attachClearButton();
264272
}
265273

266-
private createImageElements(buffer: ArrayBuffer | Uint8Array, widget: HTMLElement, hoverElement: HTMLElement, reference?: URI): void {
274+
private createImageElements(buffer: ArrayBuffer | Uint8Array, widget: HTMLElement, hoverElement: HTMLElement, reference?: URI, omittedState?: OmittedState): void {
275+
if (omittedState === OmittedState.Partial) {
276+
this.element.classList.add('partial-warning');
277+
}
278+
267279
const blob = new Blob([buffer], { type: 'image/png' });
268280
const url = URL.createObjectURL(blob);
269281
const pillImg = dom.$('img.chat-attached-context-pill-image', { src: url, alt: '' });
@@ -279,7 +291,7 @@ export class ImageAttachmentWidget extends AbstractChatAttachmentWidget {
279291
hoverElement.appendChild(imageContainer);
280292

281293
if (reference) {
282-
const urlContainer = dom.$('a.chat-attached-context-url', {}, reference.toString());
294+
const urlContainer = dom.$('a.chat-attached-context-url', {}, omittedState === OmittedState.Partial ? localize('chat.imageAttachmentWarning', "This GIF was partially omitted - current frame will be sent.") : reference.toString());
283295
const separator = dom.$('div.chat-attached-context-url-separator');
284296
this._register(dom.addDisposableListener(urlContainer, 'click', () => this.openResource(reference, false, undefined)));
285297
hoverElement.append(separator, urlContainer);

‎src/vs/workbench/contrib/chat/browser/chatContentParts/chatAttachmentsContentPart.ts‎

Lines changed: 25 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ import { fillEditorsDragData } from '../../../../browser/dnd.js';
3939
import { ResourceLabels } from '../../../../browser/labels.js';
4040
import { ResourceContextKey } from '../../../../common/contextkeys.js';
4141
import { revealInSideBarCommand } from '../../../files/browser/fileActions.contribution.js';
42-
import { IChatRequestVariableEntry, isImageVariableEntry, isPasteVariableEntry } from '../../common/chatModel.js';
42+
import { IChatRequestVariableEntry, isImageVariableEntry, isPasteVariableEntry, OmittedState } from '../../common/chatModel.js';
4343
import { ChatResponseReferencePartStatusKind, IChatContentReference } from '../../common/chatService.js';
4444
import { convertUint8ArrayToString } from '../imageUtils.js';
4545

@@ -105,7 +105,7 @@ export class ChatAttachmentsContentPart extends Disposable {
105105
ariaLabel = range ? localize('chat.fileAttachmentWithRange3', "Attached: {0}, line {1} to line {2}.", friendlyName, range.startLineNumber, range.endLineNumber) : localize('chat.fileAttachment3', "Attached: {0}.", friendlyName);
106106
}
107107

108-
if (attachment.isOmitted) {
108+
if (attachment.omittedState === OmittedState.Full) {
109109
this.customAttachment(widget, friendlyName, hoverDelegate, ariaLabel, isAttachmentOmitted);
110110
} else {
111111
const fileOptions = {
@@ -128,11 +128,17 @@ export class ChatAttachmentsContentPart extends Disposable {
128128
this.attachedContextDisposables.add(hookUpResourceAttachmentDragAndContextMenu(accessor, widget, resource));
129129
}
130130
});
131-
} else if (attachment.isImage) {
132-
ariaLabel = localize('chat.imageAttachment', "Attached image, {0}", attachment.name);
131+
} else if (isImageVariableEntry(attachment)) {
132+
if (attachment.omittedState === OmittedState.Full) {
133+
ariaLabel = localize('chat.omittedImageAttachment', "Omitted this image: {0}", attachment.name);
134+
} else if (attachment.omittedState === OmittedState.Partial) {
135+
ariaLabel = localize('chat.partiallyOmittedImageAttachment', "Partially omitted this image: {0}", attachment.name);
136+
} else {
137+
ariaLabel = localize('chat.imageAttachment', "Attached image, {0}", attachment.name);
138+
}
133139

134140
const isURL = isImageVariableEntry(attachment) && attachment.isURL;
135-
const hoverElement = this.customAttachment(widget, attachment.name, hoverDelegate, ariaLabel, isAttachmentOmitted, attachment.isImage, isURL, attachment.value as Uint8Array);
141+
const hoverElement = this.customAttachment(widget, attachment.name, hoverDelegate, ariaLabel, isAttachmentOmitted, true, isURL, attachment.value as Uint8Array);
136142

137143
const ref = attachment.references?.[0]?.reference;
138144
if (ref && URI.isUri(ref)) {
@@ -142,12 +148,10 @@ export class ChatAttachmentsContentPart extends Disposable {
142148
};
143149
this.attachedContextDisposables.add(dom.addDisposableListener(widget, 'click', clickHandler));
144150
}
145-
146-
if (!isAttachmentPartialOrOmitted) {
147-
const buffer = attachment.value as Uint8Array;
148-
this.createImageElements(buffer, widget, hoverElement, URI.isUri(ref) ? ref : undefined);
149-
this.attachedContextDisposables.add(this.hoverService.setupDelayedHover(widget, { content: hoverElement, appearance: { showPointer: true } }));
150-
}
151+
const buffer = attachment.value as Uint8Array;
152+
const omissionType = attachment.omittedState === OmittedState.Partial ? OmittedState.Partial : isAttachmentOmitted ? OmittedState.Full : undefined;
153+
this.createImageElements(buffer, widget, hoverElement, URI.isUri(ref) ? ref : undefined, omissionType);
154+
this.attachedContextDisposables.add(this.hoverService.setupDelayedHover(widget, { content: hoverElement, appearance: { showPointer: true } }));
151155
widget.style.position = 'relative';
152156
} else if (isPasteVariableEntry(attachment)) {
153157
ariaLabel = localize('chat.attachment', "Attached context, {0}", attachment.name);
@@ -244,7 +248,6 @@ export class ChatAttachmentsContentPart extends Disposable {
244248
this.attachedContextDisposables.add(this.hoverService.setupDelayedHover(widget, { content: hoverElement, appearance: { showPointer: true } }));
245249
}
246250

247-
248251
if (isAttachmentOmitted) {
249252
widget.classList.add('warning');
250253
hoverElement.textContent = localize('chat.fileAttachmentHover', "Selected model does not support this {0} type.", isImage ? 'image' : 'file');
@@ -273,7 +276,15 @@ export class ChatAttachmentsContentPart extends Disposable {
273276
}
274277

275278
// Helper function to create and replace image
276-
private createImageElements(buffer: ArrayBuffer | Uint8Array, widget: HTMLElement, hoverElement: HTMLElement, reference?: URI): void {
279+
private createImageElements(buffer: ArrayBuffer | Uint8Array, widget: HTMLElement, hoverElement: HTMLElement, reference?: URI, omittedState?: OmittedState): void {
280+
if (omittedState === OmittedState.Full) {
281+
return;
282+
}
283+
284+
if (omittedState === OmittedState.Partial) {
285+
widget.classList.add('partial-warning');
286+
}
287+
277288
const blob = new Blob([buffer], { type: 'image/png' });
278289
const url = URL.createObjectURL(blob);
279290
const pillImg = dom.$('img.chat-attached-context-pill-image', { src: url, alt: '' });
@@ -289,7 +300,7 @@ export class ChatAttachmentsContentPart extends Disposable {
289300
hoverElement.appendChild(imageContainer);
290301

291302
if (reference) {
292-
const urlContainer = dom.$('a.chat-attached-context-url', {}, reference.toString());
303+
const urlContainer = dom.$('a.chat-attached-context-url', {}, omittedState === OmittedState.Partial ? localize('chat.imageAttachmentWarning', "This GIF was partially omitted - current frame was be sent.") : reference.toString());
293304
const separator = dom.$('div.chat-attached-context-url-separator');
294305
this._register(dom.addDisposableListener(urlContainer, 'click', () => this.openResource(reference, false, undefined)));
295306
hoverElement.append(separator, urlContainer);

0 commit comments

Comments
 (0)