Fix folder deletion #140

2025-10-19T17:12:39+01:00

schmelczer commented

2025-10-19 17:12:39 +01:00

(Migrated from github.com)

Delete folders which would become empty after a syncing event

copilot-pull-request-reviewer[bot] (Migrated from github.com) reviewed 2025-10-19 17:13:17 +01:00

copilot-pull-request-reviewer[bot] (Migrated from github.com) left a comment

Pull Request Overview

This PR refactors the file listing functionality by renaming listAllFiles to listFilesRecursively and adding support for listing files within specific root directories. The changes enable more targeted file operations and implement automatic cleanup of empty parent directories when files are deleted or moved.

Renamed listAllFiles method to listFilesRecursively with optional root parameter across all implementations
Added functionality to delete empty parent directories after file deletion or move operations
Updated all method signatures and calls throughout the codebase to use the new interface

Reviewed Changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
sync-server/Dockerfile	Removed commented-out build step and deleted file copy command
frontend/test-client/src/agent/mock-client.ts	Updated method signature to include root parameter and added blank line
frontend/test-client/src/agent/mock-agent.ts	Updated method call to use new name
frontend/sync-client/src/sync-operations/syncer.ts	Updated method calls to use new name
frontend/sync-client/src/file-operations/safe-filesystem-operations.ts	Updated method signature and implementation
frontend/sync-client/src/file-operations/filesystem-operations.ts	Updated interface definition with new method signature and documentation
frontend/sync-client/src/file-operations/file-operations.ts	Added directory cleanup logic and helper methods
frontend/sync-client/src/file-operations/file-operations.test.ts	Updated mock implementation signature
frontend/obsidian-plugin/src/obsidian-file-system.ts	Updated method signature and added root parameter support

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

## Pull Request Overview This PR refactors the file listing functionality by renaming `listAllFiles` to `listFilesRecursively` and adding support for listing files within specific root directories. The changes enable more targeted file operations and implement automatic cleanup of empty parent directories when files are deleted or moved. - Renamed `listAllFiles` method to `listFilesRecursively` with optional root parameter across all implementations - Added functionality to delete empty parent directories after file deletion or move operations - Updated all method signatures and calls throughout the codebase to use the new interface ### Reviewed Changes Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments. <details> <summary>Show a summary per file</summary> | File | Description | | ---- | ----------- | | sync-server/Dockerfile | Removed commented-out build step and deleted file copy command | | frontend/test-client/src/agent/mock-client.ts | Updated method signature to include root parameter and added blank line | | frontend/test-client/src/agent/mock-agent.ts | Updated method call to use new name | | frontend/sync-client/src/sync-operations/syncer.ts | Updated method calls to use new name | | frontend/sync-client/src/file-operations/safe-filesystem-operations.ts | Updated method signature and implementation | | frontend/sync-client/src/file-operations/filesystem-operations.ts | Updated interface definition with new method signature and documentation | | frontend/sync-client/src/file-operations/file-operations.ts | Added directory cleanup logic and helper methods | | frontend/sync-client/src/file-operations/file-operations.test.ts | Updated mock implementation signature | | frontend/obsidian-plugin/src/obsidian-file-system.ts | Updated method signature and added root parameter support | </details> --- <sub>**Tip:** Customize your code reviews with copilot-instructions.md. <a href="/schmelczer/vault-link/new/main/.github?filename=copilot-instructions.md" class="Link--inTextBlock" target="_blank" rel="noopener noreferrer">Create the file</a> or <a href="https://docs.github.com/en/copilot/customizing-copilot/adding-repository-custom-instructions-for-github-copilot" class="Link--inTextBlock" target="_blank" rel="noopener noreferrer">learn how to get started</a>.</sub>

frontend/sync-client/src/file-operations/file-operations.ts

				`@ -146,6 +162,31 @@ export class FileOperations {`

copilot-pull-request-reviewer[bot] (Migrated from github.com) commented

2025-10-19 17:13:17 +01:00

The loop condition directory.length > 1 may not work correctly for relative paths. An empty directory path would have length 0, and a root directory might be represented as an empty string or single character, potentially causing the loop to continue indefinitely or skip valid directories.

		while (true) {
			const parent = require('path').dirname(directory);
			if (parent === directory) {
				break;
			}
			directory = parent;

The loop condition `directory.length > 1` may not work correctly for relative paths. An empty directory path would have length 0, and a root directory might be represented as an empty string or single character, potentially causing the loop to continue indefinitely or skip valid directories. ```suggestion while (true) { const parent = require('path').dirname(directory); if (parent === directory) { break; } directory = parent; ```

copilot-pull-request-reviewer[bot] (Migrated from github.com) commented

2025-10-19 17:13:17 +01:00

Use strict equality operator === instead of == for better type safety and consistency with modern JavaScript/TypeScript practices.

			if (remainingContent.length === 0) {

Use strict equality operator `===` instead of `==` for better type safety and consistency with modern JavaScript/TypeScript practices. ```suggestion if (remainingContent.length === 0) { ```

sync-server/Dockerfile

copilot-pull-request-reviewer[bot] (Migrated from github.com) commented

2025-10-19 17:13:16 +01:00

The file /app/test-entrypoint.sh is being made executable, but the COPY command for this file was removed on line 30. This will cause the chmod command to fail.

COPY test-entrypoint.sh /app/test-entrypoint.sh

The file `/app/test-entrypoint.sh` is being made executable, but the COPY command for this file was removed on line 30. This will cause the chmod command to fail. ```suggestion COPY test-entrypoint.sh /app/test-entrypoint.sh ```

Sign in to join this conversation.

No reviewers