Skip to content

Commit 4935bdc

Browse files
HaleshotLight2Dark
authored andcommitted
[Windows] Fix Copy Path and Copy Relative Path returning identical results. (marimo-team#5399)
## 📝 Summary Fix marimo-team#5395 ## 🔍 Description of Changes Both copy path options in the file explorer's (View File sidebar panel) context menu were returning the same absolute path on Windows systems. The relative path should show only the portion relative to the workspace root, not the full/absolute path. Fixed by replacing the slash logic with a new `withTrailingDelimiter` method that uses the existing `PathBuilder.deliminator` to append the correct path separator accordingly. Added tests covering both Unix and Windows path scenarios. **Before**: Both options returned `D:\full\path\to\file.py` **After**: Copy Path returns `D:\full\path\to\file.py`, Copy Relative Path returns `subfolder\file.py` ## Checklist - [x] I have read the [contributor guidelines](https://github.com/marimo-team/marimo/blob/main/CONTRIBUTING.md). - [ ] For large changes, or changes that affect the public API: this change was discussed or approved through an issue, on [Discord](https://marimo.io/discord?ref=pr), or the community [discussions](https://github.com/marimo-team/marimo/discussions) (Please provide a link if applicable). - [x] I have added tests for the changes made. - [x] I have run the code and verified that it works as expected. ## 📜 Reviewers @akshayka OR @Light2Dark --------- Signed-off-by: Srihari Thyagarajan <hari.leo03@gmail.com> Co-authored-by: Shahmir Varqha <sham9871@gmail.com>
1 parent 979b343 commit 4935bdc

File tree

2 files changed

+62
-5
lines changed

2 files changed

+62
-5
lines changed

frontend/src/components/editor/file-tree/__tests__/requesting-tree.test.ts

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
/* Copyright 2024 Marimo. All rights reserved. */
22
import { afterEach, beforeEach, describe, expect, test, vi } from "vitest";
33
import { toast } from "@/components/ui/use-toast";
4+
import type { FilePath } from "@/utils/paths";
45
import { RequestingTree } from "../requesting-tree";
56

67
const sendListFiles = vi.fn();
@@ -261,4 +262,60 @@ describe("RequestingTree", () => {
261262
expect(mockOnChange).toHaveBeenCalled();
262263
});
263264
});
265+
266+
describe("relativeFromRoot", () => {
267+
test("should return relative path for Unix paths", async () => {
268+
const tree = new RequestingTree({
269+
listFiles: sendListFiles,
270+
createFileOrFolder: sendCreateFileOrFolder,
271+
deleteFileOrFolder: sendDeleteFileOrFolder,
272+
renameFileOrFolder: sendRenameFileOrFolder,
273+
});
274+
275+
sendListFiles.mockResolvedValue({
276+
files: [],
277+
root: "/home/user/project",
278+
});
279+
280+
await tree.initialize(vi.fn());
281+
282+
const relativePath = tree.relativeFromRoot(
283+
"/home/user/project/src/file.py" as FilePath,
284+
);
285+
expect(relativePath).toBe("src/file.py");
286+
});
287+
288+
test("should return relative path for Windows paths", async () => {
289+
const tree = new RequestingTree({
290+
listFiles: sendListFiles,
291+
createFileOrFolder: sendCreateFileOrFolder,
292+
deleteFileOrFolder: sendDeleteFileOrFolder,
293+
renameFileOrFolder: sendRenameFileOrFolder,
294+
});
295+
296+
sendListFiles.mockResolvedValue({
297+
files: [],
298+
root: "C:\\Users\\test\\project",
299+
});
300+
301+
await tree.initialize(vi.fn());
302+
303+
const relativePath = tree.relativeFromRoot(
304+
"C:\\Users\\test\\project\\src\\file.py" as FilePath,
305+
);
306+
expect(relativePath).toBe("src\\file.py");
307+
});
308+
309+
test("should return original path when not under root", async () => {
310+
const relativePath = requestingTree.relativeFromRoot(
311+
"/other/path/file.py" as FilePath,
312+
);
313+
expect(relativePath).toBe("/other/path/file.py");
314+
});
315+
316+
test("should handle root path exactly", async () => {
317+
const relativePath = requestingTree.relativeFromRoot("/root" as FilePath);
318+
expect(relativePath).toBe("/root");
319+
});
320+
});
264321
});

frontend/src/components/editor/file-tree/requesting-tree.tsx

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -203,7 +203,11 @@ export class RequestingTree {
203203
};
204204

205205
public relativeFromRoot = (path: FilePath): FilePath => {
206-
const root = withTrailingSlash(this.rootPath);
206+
// Add a trailing delimiter to the root path if it doesn't have one
207+
const root = this.rootPath.endsWith(this.path.deliminator)
208+
? this.rootPath
209+
: `${this.rootPath}${this.path.deliminator}`;
210+
207211
if (path.startsWith(root)) {
208212
return path.slice(root.length) as FilePath;
209213
}
@@ -224,7 +228,3 @@ export class RequestingTree {
224228
return response;
225229
};
226230
}
227-
228-
function withTrailingSlash(path: string): string {
229-
return path.endsWith("/") ? path : `${path}/`;
230-
}

0 commit comments

Comments
 (0)