From c9d7aeb0c357693f83aa0a9641f15da21388b80c Mon Sep 17 00:00:00 2001 From: Bill Date: Tue, 28 Oct 2025 19:49:04 -0400 Subject: [PATCH] fix: use fileManager.trashFile instead of vault.delete Replace vault.delete() with fileManager.trashFile() to respect user's trash preferences configured in Obsidian settings. This ensures deleted files go to the user's configured trash location instead of being permanently deleted without respecting system preferences. Changes: - src/tools/note-tools.ts: Replace vault.delete with fileManager.trashFile in createNote (overwrite conflict) and deleteNote (permanent delete) - tests/note-tools.test.ts: Update test expectations to check for fileManager.trashFile calls instead of vault.delete Addresses ObsidianReviewBot required issue #3. --- src/tools/note-tools.ts | 6 +++--- tests/note-tools.test.ts | 8 ++++---- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/tools/note-tools.ts b/src/tools/note-tools.ts index f194fc0..caa0786 100644 --- a/src/tools/note-tools.ts +++ b/src/tools/note-tools.ts @@ -159,7 +159,7 @@ export class NoteTools { const existingFile = PathUtils.resolveFile(this.app, normalizedPath); /* istanbul ignore next */ if (existingFile) { - await this.vault.delete(existingFile); + await this.fileManager.trashFile(existingFile); } } else if (onConflict === 'rename') { // Generate a unique name @@ -542,8 +542,8 @@ export class NoteTools { await this.vault.trash(file, true); destination = `.trash/${file.name}`; } else { - // Permanent deletion - await this.vault.delete(file); + // Delete using user's preferred trash settings (system trash or .trash/ folder) + await this.fileManager.trashFile(file); } const result: DeleteNoteResult = { diff --git a/tests/note-tools.test.ts b/tests/note-tools.test.ts index 73c7e60..42cdfca 100644 --- a/tests/note-tools.test.ts +++ b/tests/note-tools.test.ts @@ -137,7 +137,7 @@ describe('NoteTools', () => { (PathUtils.fileExists as jest.Mock).mockReturnValue(true); (PathUtils.resolveFile as jest.Mock).mockReturnValue(mockFile); - mockVault.delete = jest.fn().mockResolvedValue(undefined); + mockFileManager.trashFile = jest.fn().mockResolvedValue(undefined); mockVault.create = jest.fn().mockResolvedValue(mockFile); (PathUtils.folderExists as jest.Mock).mockReturnValue(false); (PathUtils.getParentPath as jest.Mock).mockReturnValue(''); @@ -145,7 +145,7 @@ describe('NoteTools', () => { const result = await noteTools.createNote('test.md', 'content', false, 'overwrite'); expect(result.isError).toBeUndefined(); - expect(mockVault.delete).toHaveBeenCalledWith(mockFile); + expect(mockFileManager.trashFile).toHaveBeenCalledWith(mockFile); expect(mockVault.create).toHaveBeenCalled(); }); @@ -344,12 +344,12 @@ describe('NoteTools', () => { const mockFile = createMockTFile('test.md'); (PathUtils.resolveFile as jest.Mock).mockReturnValue(mockFile); - mockVault.delete = jest.fn().mockResolvedValue(undefined); + mockFileManager.trashFile = jest.fn().mockResolvedValue(undefined); const result = await noteTools.deleteNote('test.md', false, false); expect(result.isError).toBeUndefined(); - expect(mockVault.delete).toHaveBeenCalledWith(mockFile); + expect(mockFileManager.trashFile).toHaveBeenCalledWith(mockFile); const parsed = JSON.parse(result.content[0].text); expect(parsed.deleted).toBe(true); expect(parsed.soft).toBe(false);