-
Notifications
You must be signed in to change notification settings - Fork 3.8k
Insert file mentions when non-image files are dropped on the composer #5390
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
0xjohnnydev
wants to merge
2
commits into
pingdotgg:main
Choose a base branch
from
0xjohnnydev:feat/drop-non-image-file-mentions
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,67 @@ | ||
| import { describe, expect, it } from "@effect/vitest"; | ||
|
|
||
| import { | ||
| buildDroppedFileMentions, | ||
| partitionDroppedComposerFiles, | ||
| toComposerMentionPath, | ||
| } from "./composerFileDrop.ts"; | ||
|
|
||
| const fakeFile = (type: string, name: string): File => ({ type, name }) as unknown as File; | ||
|
|
||
| describe("partitionDroppedComposerFiles", () => { | ||
| it("splits image files from everything else, preserving order", () => { | ||
| const png = fakeFile("image/png", "shot.png"); | ||
| const doc = fakeFile("text/markdown", "notes.md"); | ||
| const jpeg = fakeFile("image/jpeg", "photo.jpg"); | ||
| const bin = fakeFile("", "Makefile"); | ||
|
|
||
| const { imageFiles, pathFiles } = partitionDroppedComposerFiles([png, doc, jpeg, bin]); | ||
|
|
||
| expect(imageFiles).toEqual([png, jpeg]); | ||
| expect(pathFiles).toEqual([doc, bin]); | ||
| }); | ||
|
|
||
| it("returns empty partitions for no files", () => { | ||
| expect(partitionDroppedComposerFiles([])).toEqual({ imageFiles: [], pathFiles: [] }); | ||
| }); | ||
| }); | ||
|
|
||
| describe("toComposerMentionPath", () => { | ||
| it("relativises a path inside the workspace cwd", () => { | ||
| expect(toComposerMentionPath("/home/me/proj/src/app.ts", "/home/me/proj")).toBe("src/app.ts"); | ||
| }); | ||
|
|
||
| it("tolerates a trailing slash on the cwd", () => { | ||
| expect(toComposerMentionPath("/home/me/proj/src/app.ts", "/home/me/proj/")).toBe("src/app.ts"); | ||
| }); | ||
|
|
||
| it("keeps the absolute path when it is outside the cwd", () => { | ||
| expect(toComposerMentionPath("/etc/hosts", "/home/me/proj")).toBe("/etc/hosts"); | ||
| }); | ||
|
|
||
| it("keeps the absolute path when there is no cwd", () => { | ||
| expect(toComposerMentionPath("/home/me/proj/src/app.ts", null)).toBe("/home/me/proj/src/app.ts"); | ||
| }); | ||
|
|
||
| it("does not relativise the cwd itself to an empty string", () => { | ||
| expect(toComposerMentionPath("/home/me/proj", "/home/me/proj")).toBe("/home/me/proj"); | ||
| }); | ||
|
|
||
| it("normalises Windows separators and relativises", () => { | ||
| expect(toComposerMentionPath("C:\\Users\\me\\proj\\src\\app.ts", "C:\\Users\\me\\proj")).toBe( | ||
| "src/app.ts", | ||
| ); | ||
| }); | ||
| }); | ||
|
|
||
| describe("buildDroppedFileMentions", () => { | ||
| it("serializes each path as a space-separated file link", () => { | ||
| expect( | ||
| buildDroppedFileMentions(["/home/me/proj/src/app.ts", "/etc/hosts"], "/home/me/proj"), | ||
| ).toBe("[app.ts](src/app.ts) [hosts](/etc/hosts)"); | ||
| }); | ||
|
|
||
| it("returns an empty string for no paths", () => { | ||
| expect(buildDroppedFileMentions([], "/home/me/proj")).toBe(""); | ||
| }); | ||
| }); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,59 @@ | ||
| import { serializeComposerFileLink } from "@t3tools/shared/composerTrigger"; | ||
|
|
||
| /** | ||
| * Split files from an OS drag-and-drop into the two ways the composer can use | ||
| * them: images become inline attachments, everything else becomes a file | ||
| * mention that points the agent at the dropped path. | ||
| */ | ||
| export interface DroppedComposerFilePartition { | ||
| readonly imageFiles: File[]; | ||
| readonly pathFiles: File[]; | ||
| } | ||
|
|
||
| export function partitionDroppedComposerFiles( | ||
| files: readonly File[], | ||
| ): DroppedComposerFilePartition { | ||
| const imageFiles: File[] = []; | ||
| const pathFiles: File[] = []; | ||
| for (const file of files) { | ||
| if (file.type.startsWith("image/")) { | ||
| imageFiles.push(file); | ||
| } else { | ||
| pathFiles.push(file); | ||
| } | ||
| } | ||
| return { imageFiles, pathFiles }; | ||
| } | ||
|
|
||
| /** | ||
| * Convert an absolute filesystem path into the value inserted as a composer | ||
| * mention. When the path lives inside the workspace cwd it is made | ||
| * workspace-relative so it matches typed and file-tree mentions; otherwise the | ||
| * absolute path is kept so the reference is still unambiguous. | ||
| * | ||
| * Path separators are normalised to `/` so Windows drops relativise too. The | ||
| * prefix comparison is case-sensitive, so a drop that differs only in casing | ||
| * from the cwd (possible on case-insensitive volumes) keeps its absolute path | ||
| * rather than guessing a relative one. | ||
| */ | ||
| export function toComposerMentionPath(absolutePath: string, cwd: string | null): string { | ||
| const normalizedPath = absolutePath.replace(/\\/g, "/"); | ||
| if (cwd !== null) { | ||
| const normalizedCwd = cwd.replace(/\\/g, "/").replace(/\/+$/, ""); | ||
| if (normalizedCwd.length > 0) { | ||
| const prefix = `${normalizedCwd}/`; | ||
| if (normalizedPath.startsWith(prefix) && normalizedPath.length > prefix.length) { | ||
| return normalizedPath.slice(prefix.length); | ||
| } | ||
| } | ||
| } | ||
| return normalizedPath; | ||
| } | ||
|
|
||
| /** | ||
| * Build the composer text for a set of dropped file paths: one serialized file | ||
| * link per path, space-separated so each stays a valid mention token. | ||
| */ | ||
| export function buildDroppedFileMentions(paths: readonly string[], cwd: string | null): string { | ||
| return paths.map((path) => serializeComposerFileLink(toComposerMentionPath(path, cwd))).join(" "); | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🟡 Medium
chat/composerFileDrop.ts:39toComposerMentionPathstrips all trailing slashes fromcwd, so whencwdis"/"the normalized cwd becomes an empty string and the relativization branch is skipped entirely. Dropping/src/app.tswith the workspace rooted at/returns/src/app.tsinstead of the expectedsrc/app.ts. Consider preserving POSIX root"/"as the prefix when normalizing instead of stripping it down to an empty string.export function toComposerMentionPath(absolutePath: string, cwd: string | null): string { const normalizedPath = absolutePath.replace(/\\/g, "/"); if (cwd !== null) { - const normalizedCwd = cwd.replace(/\\/g, "/").replace(/\/+$/, ""); + const normalizedCwd = + cwd.replace(/\\/g, "/") === "/" + ? "/" + : cwd.replace(/\\/g, "/").replace(/\/+$/, ""); if (normalizedCwd.length > 0) {🤖 Copy this AI Prompt to have your agent fix this: