Skip to content

Commit 43f0dca

Browse files
committed
Improve code
1 parent 4932fd8 commit 43f0dca

2 files changed

Lines changed: 28 additions & 20 deletions

File tree

‎TODO.md‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,14 @@ Track progress of implementing Linux `tree` options in tree-node-cli.
5656
- CLI: `-J, --json`
5757
- API: `json: boolean`
5858

59+
## Code Improvements
60+
61+
- [x] **Filter before stat** — `statEntries` stats all entries including hidden files that will be filtered out when `allFiles: false`. Filter hidden files before stat'ing to avoid unnecessary syscalls.
62+
- [ ] **Match Linux `tree -s` size behavior** — `computeSizes` recursively sums file sizes into parent directories, but Linux `tree -s` shows each entry's own `stat.size` (inode size for directories, not recursive sum). Remove `computeSizes` and use `stat.size` directly.
63+
- [x] **Merge `EXCLUDED_PATTERNS` into exclude** — The hardcoded `.DS_Store` pattern is checked in a separate loop for every entry. Merge it into the user's `exclude` array at initialization to simplify `isExcluded`.
64+
- [x] **Clarify `dirsOnly` filtering** — `dirsOnly` is checked in both `isExcluded` (skips files early) and `filterAndOrderContents` (filters file entries). The `isExcluded` check is a performance optimization that avoids stat'ing files, but the dual responsibility could be clearer.
65+
- [x] **Avoid mutating input arrays** — `sortContents` calls `contents.sort()` which mutates the caller's array in place. Using `[...contents].sort(...)` would be safer, though not currently a bug since callers don't reuse the array.
66+
5967
## Notes
6068

6169
- Each feature should include: implementation in `src/index.ts`, CLI flag in `src/cli.ts`, tests in `src/index.test.ts` and `src/cli.test.ts`, and README docs

‎src/index.ts‎

Lines changed: 20 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ const DEFAULT_OPTIONS: RequiredOptions = {
5050
unsorted: false,
5151
};
5252

53-
const EXCLUDED_PATTERNS = [/\.DS_Store/];
53+
const DEFAULT_EXCLUDE_PATTERNS: RegExp[] = [/\.DS_Store/];
5454

5555
// ── Tree node ──────────────────────────────────────────────
5656

@@ -72,20 +72,9 @@ function isHiddenFile(filename: string): boolean {
7272

7373
function isExcluded(
7474
path: string,
75-
isFile: boolean,
7675
currentDepth: number,
7776
options: RequiredOptions,
7877
): boolean {
79-
for (const pattern of EXCLUDED_PATTERNS) {
80-
if (pattern.test(path)) {
81-
return true;
82-
}
83-
}
84-
85-
if (isFile && options.dirsOnly) {
86-
return true;
87-
}
88-
8978
for (const pattern of options.exclude) {
9079
if (pattern.test(path)) {
9180
return true;
@@ -112,6 +101,7 @@ function statEntries(
112101
return stats;
113102
}
114103

104+
// Sorts `contents` in place. Callers must not reuse the input array.
115105
function sortContents(
116106
contents: string[],
117107
stats: Map<string, fs.Stats>,
@@ -156,10 +146,6 @@ function filterAndOrderContents(
156146
stats: Map<string, fs.Stats>,
157147
options: RequiredOptions,
158148
): string[] {
159-
if (!options.allFiles) {
160-
contents = contents.filter((content) => !isHiddenFile(content));
161-
}
162-
163149
if (!options.dirsOnly && !options.dirsFirst && !options.filesFirst) {
164150
return contents;
165151
}
@@ -187,7 +173,13 @@ function getDirectoryContents(
187173
dirPath: string,
188174
options: RequiredOptions,
189175
): { entries: string[]; stats: Map<string, fs.Stats> } {
190-
const entries = fs.readdirSync(dirPath);
176+
let entries = fs.readdirSync(dirPath);
177+
178+
// Filter hidden files before stat'ing to avoid unnecessary syscalls.
179+
if (!options.allFiles) {
180+
entries = entries.filter((entry) => !isHiddenFile(entry));
181+
}
182+
191183
const stats = statEntries(dirPath, entries);
192184
const sorted = sortContents(entries, stats, options);
193185
const filtered = filterAndOrderContents(sorted, stats, options);
@@ -227,7 +219,7 @@ function buildTree(
227219
const isDir = stat.isDirectory();
228220
const isFile = !isDir;
229221

230-
if (isExcluded(path, isFile, currentDepth, options)) {
222+
if (isExcluded(path, currentDepth, options)) {
231223
return null;
232224
}
233225

@@ -287,7 +279,11 @@ function buildTreeNode(
287279
}
288280

289281
export function tree(path: string, options?: Options): string {
290-
const combinedOptions: RequiredOptions = { ...DEFAULT_OPTIONS, ...options };
282+
const combinedOptions: RequiredOptions = {
283+
...DEFAULT_OPTIONS,
284+
...options,
285+
exclude: [...DEFAULT_EXCLUDE_PATTERNS, ...(options?.exclude ?? [])],
286+
};
291287
const root = buildTreeNode(path, combinedOptions);
292288

293289
if (!root) {
@@ -301,7 +297,11 @@ export function treeJson(
301297
path: string,
302298
options?: Options,
303299
): TreeJsonEntry | null {
304-
const combinedOptions: RequiredOptions = { ...DEFAULT_OPTIONS, ...options };
300+
const combinedOptions: RequiredOptions = {
301+
...DEFAULT_OPTIONS,
302+
...options,
303+
exclude: [...DEFAULT_EXCLUDE_PATTERNS, ...(options?.exclude ?? [])],
304+
};
305305
const root = buildTreeNode(path, combinedOptions);
306306

307307
return root ? formatAsJson(root, combinedOptions) : null;

0 commit comments

Comments
 (0)