-
Notifications
You must be signed in to change notification settings - Fork 5
fix: code quality enhancements and UI improvements (#23) #24
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.
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
Changes from all commits
31ac918
a77e015
c945b9d
c6152a6
7f223e3
8f7207a
556f9e9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,71 +1,137 @@ | ||
| import ignore, { Ignore } from 'ignore'; | ||
| import { App } from 'obsidian'; | ||
| import { GitServiceInterface } from '../services/git-service-interface'; | ||
| import { logger } from '../utils/logger'; | ||
|
|
||
| export class GitignoreManager { | ||
| private readonly app: App; | ||
| private readonly gitService: GitServiceInterface; | ||
| private readonly branch: string; | ||
|
|
||
| private readonly rootPath: string; | ||
| private readonly vaultFolder: string; | ||
|
|
||
| // Maps directory path (empty string for root) to Ignore instance | ||
| private readonly ignoreMap: Map<string, Ignore> = new Map(); | ||
|
|
||
| constructor(app: App, gitService: GitServiceInterface, branch: string, rootPath: string) { | ||
| constructor(app: App, gitService: GitServiceInterface, branch: string, rootPath: string, vaultFolder: string = '') { | ||
| this.app = app; | ||
| this.gitService = gitService; | ||
| this.branch = branch; | ||
| this.rootPath = rootPath.replace(/^\/|\/$/g, ''); | ||
| this.vaultFolder = vaultFolder.replace(/^\/|\/$/g, ''); | ||
| } | ||
|
|
||
| private getNormalizedPath(path: string): string { | ||
| if (!this.vaultFolder) return path; | ||
| const folderPath = this.vaultFolder + '/'; | ||
| if (path.startsWith(folderPath)) { | ||
| return path.substring(folderPath.length); | ||
| } | ||
| if (path === this.vaultFolder) return ''; | ||
| return path; | ||
| } | ||
|
|
||
| /** | ||
| * Discovers and parses .gitignore files from the local filesystem and remote repository. | ||
| * Local files take priority; remote supplements anything not found locally. | ||
| */ | ||
| async loadGitignores(): Promise<void> { | ||
| this.ignoreMap.clear(); | ||
|
|
||
| // 1. Fetch all gitignore paths from the entire repo tree | ||
| let gitignorePaths: string[] = []; | ||
| // 1. Collect all potential gitignore paths | ||
| const gitignorePaths = new Set<string>(); | ||
|
|
||
| // a. Repo root | ||
| gitignorePaths.add('.gitignore'); | ||
|
|
||
| // b. All parent directories of rootPath | ||
| if (this.rootPath) { | ||
| const parts = this.rootPath.split('/'); | ||
| let current = ''; | ||
| for (const part of parts) { | ||
| if (current) current += '/'; | ||
| current += part; | ||
| gitignorePaths.add(current + '/.gitignore'); | ||
| } | ||
| } | ||
|
|
||
| // c. Scan local vault for .gitignore files (vault-relative → repo-relative) | ||
| await this.scanLocalGitignores(gitignorePaths); | ||
|
|
||
| // d. Supplement with remote repo's gitignore listing (filtered to rootPath) | ||
| try { | ||
| gitignorePaths = await this.gitService.getRepoGitignores(this.branch); | ||
| const remotePaths = await this.gitService.getRepoGitignores(this.branch); | ||
| for (const p of remotePaths) gitignorePaths.add(p); | ||
| } catch (e) { | ||
| console.warn('Failed to fetch repo gitignores', e); | ||
| // Fallback to at least checking the root | ||
| gitignorePaths = ['.gitignore']; | ||
| logger.warn('Failed to fetch repo gitignores', e); | ||
| } | ||
|
|
||
| // 2. Fetch and parse each .gitignore | ||
| // 2. Load content and build ignore instances | ||
| for (const fullGitignorePath of gitignorePaths) { | ||
| const dirPath = fullGitignorePath === '.gitignore' ? '' : fullGitignorePath.slice(0, -('.gitignore'.length + 1)); | ||
| const dirPath = fullGitignorePath === '.gitignore' | ||
| ? '' | ||
| : fullGitignorePath.slice(0, -(('.gitignore'.length) + 1)); | ||
| const content = await this.getGitignoreContent(fullGitignorePath); | ||
|
|
||
| if (content) { | ||
| const ig = ignore().add(content); | ||
| this.ignoreMap.set(dirPath, ig); | ||
| this.ignoreMap.set(dirPath, ignore().add(content)); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| private async scanLocalGitignores(out: Set<string>): Promise<void> { | ||
| // Only scan within vaultFolder | ||
| await this.scanDir(this.vaultFolder, out); | ||
| } | ||
|
|
||
| private async scanDir(vaultDir: string, out: Set<string>): Promise<void> { | ||
| try { | ||
| const listing = await this.app.vault.adapter.list(vaultDir || ''); | ||
| for (const filePath of listing.files) { | ||
| if (filePath === '.gitignore' || filePath.endsWith('/.gitignore')) { | ||
| const normalized = this.getNormalizedPath(filePath); | ||
| const repoPath = this.rootPath ? `${this.rootPath}/${normalized}` : normalized; | ||
| out.add(repoPath); | ||
| } | ||
| } | ||
| for (const subFolder of listing.folders) { | ||
| await this.scanDir(subFolder, out); | ||
| } | ||
| } catch { /* adapter.list may be unavailable in some environments */ } | ||
| } | ||
|
|
||
| private getVaultPath(normalizedPath: string): string { | ||
| if (!this.vaultFolder) return normalizedPath; | ||
| if (!normalizedPath) return this.vaultFolder; | ||
| return this.vaultFolder + '/' + normalizedPath; | ||
| } | ||
|
|
||
| private async getGitignoreContent(fullGitignorePath: string): Promise<string | undefined> { | ||
| let content: string | undefined; | ||
|
|
||
| // Determine local path relative to vault root | ||
| let localPath: string | null = null; | ||
| let normalized: string | null; | ||
| if (!this.rootPath) { | ||
| localPath = fullGitignorePath; | ||
| normalized = fullGitignorePath; | ||
| } else if (fullGitignorePath === this.rootPath + '/.gitignore' || fullGitignorePath.startsWith(this.rootPath + '/')) { | ||
| localPath = fullGitignorePath.substring(this.rootPath.length + 1); | ||
| normalized = fullGitignorePath.substring(this.rootPath.length + 1); | ||
| } else if (fullGitignorePath === '.gitignore') { | ||
| // Repo root gitignore might not be in the vault sync area | ||
| normalized = null; | ||
| } else { | ||
| normalized = null; | ||
| } | ||
|
|
||
| const localPath = normalized !== null ? this.getVaultPath(normalized) : null; | ||
|
Check warning on line 125 in src/logic/gitignore-manager.ts
|
||
|
|
||
| // Try local first if it's within the vault | ||
| if (localPath) { | ||
| try { | ||
| if (await this.app.vault.adapter.exists(localPath)) { | ||
| content = await this.app.vault.adapter.read(localPath); | ||
| } | ||
| } catch (e) { | ||
| console.warn(`Failed to read local ${localPath}`, e); | ||
| logger.warn(`Failed to read local ${localPath}`, e); | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -74,7 +140,7 @@ | |
| try { | ||
| const remoteFile = await this.gitService.getFile('/' + fullGitignorePath, this.branch); | ||
| if (remoteFile?.content) { | ||
| content = remoteFile.content; | ||
| content = remoteFile.content as string; | ||
| } | ||
|
Comment on lines
142
to
144
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is an unsafe type cast on line 143. If Consider something like this: if (remoteFile?.content) {
if (typeof remoteFile.content === 'string') {
content = remoteFile.content;
} else {
logger.warn(`Received unexpected binary content for gitignore file: ${fullGitignorePath}`);
}
} |
||
| } catch { | ||
| // It's okay if some gitignores fail to fetch | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Silently catching and ignoring errors can hide underlying problems. It's better to at least log a warning here so that if
adapter.listfails for an unexpected reason, it can be debugged.