Skip to content

feat: add file deletion feature to menu actions #78

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.

Sign up for GitHub

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

Draft
nyatinte wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

feat: add file deletion feature to menu actions #78

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,7 @@ ccexp --path ~/workspace # Specific directory
- **d** - Copy file to current directory (in menu)
- **e** - Edit file with $EDITOR (in menu)
- **o** - Open file in default application (in menu)
- **x** - Delete file (in menu, requires confirmation)

## Development

Expand Down
1 change: 1 addition & 0 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@
"lefthook": "^1.12.2",
"publint": "^0.3.12",
"tsdown": "^0.12.9",
"typescript": "^5.9.3",
"vitest": "^3.2.4"
},
"overrides": {
Expand Down
8 changes: 7 additions & 1 deletion src/components/FileList/MenuActions.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ if (import.meta.vitest) {
expect(lastFrame()).toContain('[D] Copy to Current Directory');
expect(lastFrame()).toContain('[E] Edit File');
expect(lastFrame()).toContain('[O] Open File');
expect(lastFrame()).toContain('[X] Delete File');
Copy link

@coderabbitai coderabbitai bot Nov 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion | 🟠 Major

Add functional tests for the delete action and confirmation flow.

The current tests only verify UI rendering. Essential tests are missing for the core delete functionality, confirmation dialog interaction, and error handling.

Consider adding these test cases:

  1. Confirmation dialog display test: Verify that pressing 'x' triggers the confirmation dialog
  2. Successful deletion test: Verify file is deleted after confirming
  3. Deletion cancellation test: Verify file is NOT deleted after canceling
  4. Error handling tests: Test permission errors and file-not-found scenarios
  5. Post-deletion behavior: Verify the menu closes or refreshes after deletion

Example test structure:

test('delete file shows confirmation dialog', async () => {
  await withCachedReadOnlyFixture(
    {
      'test-project': {
        'test.md': '# Test',
      },
    },
    async (fixture) => {
      const file = createFileInfo(
        fixture.path,
        'test-project/test.md',
        'project-memory',
      );
      const onClose = vi.fn();

      const { stdin, lastFrame } = render(
        <MenuActions file={file} onClose={onClose} />,
      );

      // Trigger delete action
      stdin.write('x');

      await waitFor(() => {
        const output = lastFrame();
        if (!output || !output.includes('Delete "test.md"?')) {
          throw new Error('Confirmation dialog not shown');
        }
      }, 100);

      // Verify confirmation message
      expect(lastFrame()).toContain('Delete "test.md"?');
      expect(lastFrame()).toContain('cannot be undone');
    },
  );
});

test('delete file removes file after confirmation', async () => {
  // Test actual file deletion after Y confirmation
});

test('delete file cancels when user declines', async () => {
  // Test file is NOT deleted after N/Esc
});

Would you like me to generate complete test implementations for the delete functionality?

🤖 Prompt for AI Agents 
In src/components/FileList/MenuActions.test.tsx around line 57, the tests only
assert UI rendering and lack functional coverage for the delete action and
confirmation flow; add tests that (1) press 'x' to open the confirmation dialog
and assert lastFrame() contains 'Delete "filename"?' and 'cannot be undone', (2)
simulate confirming deletion (write 'y' or Enter) and verify the file is removed
from the fixture (use withCachedReadOnlyFixture and createFileInfo) and any
expected UI/menu refresh or onClose is called, (3) simulate cancelling deletion
('n' or Escape) and assert the file still exists and menu remains/open state is
unchanged, (4) add error-handling tests by mocking the delete operation to throw
permission or not-found errors and assert appropriate error messages are shown,
and (5) assert post-deletion behavior such as the menu closing or refresh by
checking onClose was called or lastFrame() no longer shows the deleted file; for
all tests use render to obtain stdin/lastFrame, waitFor to poll UI changes, and
inspect the fixture filesystem or mocked delete calls to confirm side effects.

},
);
});
Expand Down Expand Up @@ -157,13 +158,14 @@ if (import.meta.vitest) {
<MenuActions file={file} onClose={onClose} />,
);

// Verify 6 actions are present
// Verify 7 actions are present
expect(lastFrame()).toContain('[C] Copy Content');
expect(lastFrame()).toContain('[P] Copy Path (Absolute)');
expect(lastFrame()).toContain('[R] Copy Path (Relative)');
expect(lastFrame()).toContain('[D] Copy to Current Directory');
expect(lastFrame()).toContain('[E] Edit File');
expect(lastFrame()).toContain('[O] Open File');
expect(lastFrame()).toContain('[X] Delete File');
},
);
});
Expand Down Expand Up @@ -225,13 +227,15 @@ if (import.meta.vitest) {
output?.indexOf('[D] Copy to Current Directory') ?? -1;
const editFileIndex = output?.indexOf('[E] Edit File') ?? -1;
const openFileIndex = output?.indexOf('[O] Open File') ?? -1;
const deleteFileIndex = output?.indexOf('[X] Delete File') ?? -1;

expect(copyContentIndex).toBeGreaterThan(-1);
expect(copyAbsoluteIndex).toBeGreaterThan(copyContentIndex);
expect(copyRelativeIndex).toBeGreaterThan(copyAbsoluteIndex);
expect(copyDirIndex).toBeGreaterThan(copyRelativeIndex);
expect(editFileIndex).toBeGreaterThan(copyDirIndex);
expect(openFileIndex).toBeGreaterThan(editFileIndex);
expect(deleteFileIndex).toBeGreaterThan(openFileIndex);
},
);
});
Expand Down Expand Up @@ -378,6 +382,7 @@ if (import.meta.vitest) {
expect(output).toContain('Copy to Current Directory');
expect(output).toContain('Edit File');
expect(output).toContain('Open File');
expect(output).toContain('Delete File');
},
);
});
Expand Down Expand Up @@ -410,6 +415,7 @@ if (import.meta.vitest) {
expect(output).toContain('[D]');
expect(output).toContain('[E]');
expect(output).toContain('[O]');
expect(output).toContain('[X]');
},
);
});
Expand Down
24 changes: 24 additions & 0 deletions src/components/FileList/MenuActions/hooks/useMenu.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,30 @@ export const useMenu = ({ file, onClose }: UseMenuProps) => {
return '✅ File opened';
},
},
{
key: 'x',
label: 'Delete File',
description: 'Delete file (requires confirmation)',
action: async () => {
const fs = await import('node:fs/promises');

// The actual deletion operation
const performDelete = async () => {
await fs.unlink(file.path);
return `✅ File deleted: ${basename(file.path)}`;
};

// Always require confirmation for deletion
setConfirmMessage(
`Are you sure you want to delete "${basename(file.path)}"? This action cannot be undone.`,
);
setIsConfirming(true);
setPendingAction(() => performDelete);

// Return early, action will be executed after confirmation
return '';
},
},
Comment on lines +230 to +253
Copy link

@coderabbitai coderabbitai bot Nov 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Critical: Menu remains open after file deletion, showing a non-existent file.

After successfully deleting a file, the menu remains open and continues to display the now-deleted file. This creates a confusing UX where users can attempt further actions on a file that no longer exists, leading to ENOENT errors.

Consider one of these solutions:

Solution 1 (recommended): Close the menu immediately after successful deletion

Modify the performDelete function to trigger menu closure:

 const performDelete = async () => {
   await fs.unlink(file.path);
+  onClose(); // Close menu immediately after deletion
   return `✅ File deleted: ${basename(file.path)}`;
 };

Solution 2: Add a callback to refresh the parent file list

If the parent component maintains a file list that should be refreshed:

 type UseMenuProps = {
   readonly file: ClaudeFileInfo;
   readonly onClose: () => void;
+  readonly onFileDeleted?: (path: string) => void;
 };

Then call it after deletion:

 const performDelete = async () => {
   await fs.unlink(file.path);
+  onFileDeleted?.(file.path);
+  onClose();
   return `✅ File deleted: ${basename(file.path)}`;
 };
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
{
key: 'x',
label: 'Delete File',
description: 'Delete file (requires confirmation)',
action: async () => {
const fs = await import('node:fs/promises');
// The actual deletion operation
const performDelete = async () => {
await fs.unlink(file.path);
return `✅ File deleted: ${basename(file.path)}`;
};
// Always require confirmation for deletion
setConfirmMessage(
`Are you sure you want to delete "${basename(file.path)}"? This action cannot be undone.`,
);
setIsConfirming(true);
setPendingAction(() => performDelete);
// Return early, action will be executed after confirmation
return '';
},
},
{
key: 'x',
label: 'Delete File',
description: 'Delete file (requires confirmation)',
action: async () => {
const fs = await import('node:fs/promises');
// The actual deletion operation
const performDelete = async () => {
await fs.unlink(file.path);
onClose(); // Close menu immediately after deletion
return `✅ File deleted: ${basename(file.path)}`;
};
// Always require confirmation for deletion
setConfirmMessage(
`Are you sure you want to delete "${basename(file.path)}"? This action cannot be undone.`,
);
setIsConfirming(true);
setPendingAction(() => performDelete);
// Return early, action will be executed after confirmation
return '';
},
},

],
[file.path, file.type, copyToClipboard, openFile, editFile],
);
Expand Down
Loading