Skip to content

feat: support hidden file sync and expand binary extension list - #25

Merged
ClaudiaFang merged 9 commits into
masterfrom
issue-23-code-quality
May 26, 2026
Merged

ClaudiaFang merged 9 commits into
masterfrom
issue-23-code-quality

Conversation

@ClaudiaFang

Copy link
Copy Markdown
Member

Summary

  • Use FileSystemAdapter.list() recursively to discover hidden files (e.g. .claude) instead of vault.getFiles()
  • Fix ensureParentDirs() to use adapter.mkdir() so hidden directories can be created on pull
  • Expand BINARY_EXTENSIONS with 20+ modern formats (heic, avif, flac, mkv, sqlite, psd, etc.)
  • Exclude .agents/** from ESLint to fix parse errors on skill reference files

Test plan

  • Verify .claude/ files appear in sync status view
  • Verify pull creates hidden directories correctly
  • Verify binary detection works for new formats
  • Confirm npm run lint passes

Generated with Claude Code

ClaudiaFang and others added 2 commits May 22, 2026 03:08
- Use FileSystemAdapter.list() recursively to discover hidden files (e.g. .claude)
- Fix ensureParentDirs to use adapter.mkdir() for hidden directory creation
- Expand BINARY_EXTENSIONS with modern image, audio, video, archive, and design formats
- Exclude .agents/** from ESLint to prevent project-service parse errors

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review

This pull request removes documentation and template files, updates the ESLint configuration to ignore agent-related directories, and significantly expands the list of recognized binary file extensions. Key logic changes include switching to the vault adapter for file listing and directory creation. Feedback suggests refining the directory creation logic to preserve Obsidian's internal indexing for non-hidden folders and optimizing the recursive file listing to prevent potential stack overflow errors and improve performance.

Comment thread src/logic/sync-manager.ts
Comment on lines +263 to 267
try {
await this.app.vault.adapter.mkdir(cur);
} catch {
// already exists or failed
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

Using adapter.mkdir for all directories bypasses Obsidian's internal file index for regular (non-hidden) folders. This can lead to a delay in the folders appearing in the UI and prevents Obsidian from triggering relevant events. Additionally, calling mkdir without checking if the folder exists first is less efficient as it relies on catching exceptions for existing directories.

Consider using vault.getAbstractFileByPath to check for existence first, and only use adapter.mkdir for paths that contain hidden segments (starting with .), falling back to vault.createFolder for regular directories.

Suggested change
try {
await this.app.vault.adapter.mkdir(cur);
} catch {
// already exists or failed
}
if (!this.app.vault.getAbstractFileByPath(cur)) {
try {
if (cur.split('/').some(part => part.startsWith('.'))) {
await this.app.vault.adapter.mkdir(cur);
} else {
await this.app.vault.createFolder(cur);
}
} catch {
// already exists or failed
}
}

Comment thread src/main.ts
Comment on lines +141 to +152
private async listAllFilesFromAdapter(dirPath: string): Promise<string[]> {
const results: string[] = [];
try {
const { files, folders } = await this.app.vault.adapter.list(dirPath || '');
results.push(...files);
for (const folder of folders) {
const sub = await this.listAllFilesFromAdapter(folder);
results.push(...sub);
}
} catch { /* ignore inaccessible dirs */ }
return results;
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

The recursive implementation of listAllFilesFromAdapter has two potential issues:

  1. Performance/Memory: It creates a new array for every folder in the vault and uses the spread operator (...) to merge them. For large vaults, this results in many intermediate array allocations and O(N^2) complexity in terms of element copies.
  2. Crash Risk: The spread operator in a function call (results.push(...sub)) is limited by the JavaScript engine's argument stack size. If a subtree contains a very large number of files (typically >65k), this will throw a RangeError.

Using an accumulator pattern with a simple loop is more efficient and avoids the stack size limit.

Suggested change
private async listAllFilesFromAdapter(dirPath: string): Promise<string[]> {
const results: string[] = [];
try {
const { files, folders } = await this.app.vault.adapter.list(dirPath || '');
results.push(...files);
for (const folder of folders) {
const sub = await this.listAllFilesFromAdapter(folder);
results.push(...sub);
}
} catch { /* ignore inaccessible dirs */ }
return results;
}
private async listAllFilesFromAdapter(dirPath: string, results: string[] = []): Promise<string[]> {
try {
const { files, folders } = await this.app.vault.adapter.list(dirPath || '');
for (const file of files) {
results.push(file);
}
for (const folder of folders) {
await this.listAllFilesFromAdapter(folder, results);
}
} catch { /* ignore inaccessible dirs */ }
return results;
}

ClaudiaFang and others added 7 commits May 22, 2026 03:16
- Add sync-manager-binary.test.ts: push/pull ArrayBuffer via adapter.readBinary/writeBinary
- Add sync-manager-hidden.test.ts: hidden path mkdir, push, pull via string paths
- Add utils/path.test.ts: isBinaryPath and contentsEqual full coverage
- Add metadata-on-equal assertion to sync-manager tests
- Add GitHub truncated result and GitLab pagination boundary tests
- Add hidden file gitignore filter tests
- Add docs/test-coverage.md with all 189 test cases documented

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…cation

Move repeated beforeEach mock initialization into createSyncManagerMocks()
helper to eliminate ~45 lines of copy-paste across binary and hidden test files.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Extract adapter variable and loadWith() helper in gitignore hidden-file tests
- Hoist shared ArrayBuffer constants in path.test.ts
Reduces new_duplicated_lines_density from ~78%/46% to near 0%.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@sonarqubecloud

Copy link
Copy Markdown

Quality Gate Failed Quality Gate failed

Failed conditions
17.4% Duplication on New Code (required ≤ 3%)

See analysis details on SonarQube Cloud

@ClaudiaFang
ClaudiaFang merged commit eefd728 into master May 26, 2026
18 of 22 checks passed
@ClaudiaFang
ClaudiaFang deleted the issue-23-code-quality branch May 26, 2026 04:51
@ClaudiaFang

Copy link
Copy Markdown
Member Author

🎉 This PR is included in version 1.1.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant