fix: Windows compatibility for delete, size, and path separators #7
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,44 @@ | ||
| name: CI | ||
|
|
||
| on: | ||
| push: | ||
| branches: [main] | ||
| pull_request: | ||
| branches: [main] | ||
|
|
||
| jobs: | ||
| test: | ||
| name: Test on ${{ matrix.os }} | ||
| runs-on: ${{ matrix.os }} | ||
| strategy: | ||
| fail-fast: false | ||
| matrix: | ||
| os: [ubuntu-latest, macos-latest, windows-latest] | ||
|
|
||
| steps: | ||
| - uses: actions/checkout@v4 | ||
|
|
||
| - uses: oven-sh/setup-bun@v2 | ||
| with: | ||
| bun-version: latest | ||
|
|
||
| - name: Install dependencies | ||
| run: bun install | ||
|
|
||
| - name: Type check | ||
| run: bun run check | ||
|
|
||
| - name: Build | ||
| run: bun run build | ||
|
|
||
| - name: Smoke test - help | ||
| run: bun run src/cli.ts --help | ||
|
|
||
| - name: Smoke test - dry-run (Unix) | ||
| if: runner.os != 'Windows' | ||
| run: bun run src/cli.ts --dir ${{ github.workspace }} --dry-run --hide-errors | ||
|
|
||
| - name: Smoke test - dry-run (Windows) | ||
| if: runner.os == 'Windows' | ||
| run: bun run src/cli.ts --dir ${{ github.workspace }} --dry-run --hide-errors | ||
| shell: pwsh | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -29,15 +29,22 @@ BunKill scans large directory trees, calculates folder sizes, and lets you delet | |
|
|
||
| ## Requirements | ||
|
|
||
| - [Bun](https://bun.sh) runtime is required | ||
| - Supported platforms: **macOS**, **Linux**, **Windows 10/11** | ||
| - Windows: requires [Windows Terminal](https://aka.ms/terminal) for best interactive UI experience | ||
|
|
||
| **Install Bun:** | ||
|
|
||
| macOS / Linux: | ||
| ```bash | ||
| curl -fsSL https://bun.sh/install | bash | ||
| ``` | ||
|
|
||
| Windows (PowerShell): | ||
| ```powershell | ||
| powershell -c "irm bun.sh/install.ps1 | iex" | ||
| ``` | ||
|
|
||
| ## Install | ||
|
|
||
| ```bash | ||
|
|
@@ -56,9 +63,12 @@ bun install -g bunkill | |
| # interactive scan in current directory | ||
| bunkill | ||
|
|
||
| # scan a specific directory (macOS / Linux) | ||
| bunkill --dir ~/Projects | ||
|
|
||
| # scan a specific directory (Windows) | ||
| bunkill --dir "C:\Users\YourName\Projects" | ||
|
|
||
| # preview only | ||
| bunkill --dir ~/Projects --dry-run | ||
|
|
||
|
|
@@ -101,13 +111,14 @@ Search filters the already loaded list, so you can quickly narrow large result s | |
|
|
||
| ## Platform status | ||
|
|
||
| | Platform | Status | | ||
| |---|---| | ||
| | macOS | ✅ Tested | | ||
| | Linux | ⚠️ Not tested yet | | ||
| | Windows 10/11 | ✅ Tested (Windows Terminal recommended) | | ||
|
Comment on lines
+114
to
+118
There was a problem hiding this comment. Align Linux platform status with the new CI matrix. Line 117 currently says Linux is “Not tested yet”, but this PR adds Linux CI runs. Please update wording to avoid contradictory platform claims. 🤖 Prompt for AI Agents
Comment on lines
+114
to
+118
There was a problem hiding this comment. README now claims Linux is " |
||
|
|
||
| > **Windows note:** Interactive mode requires a terminal that supports ANSI escape codes and raw mode. | ||
| > [Windows Terminal](https://aka.ms/terminal) works well. The legacy `cmd.exe` prompt is not supported. | ||
|
|
||
| ## Performance | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -34,10 +34,24 @@ function shouldSkip(p: string): boolean { | |
| ); | ||
| } | ||
|
|
||
| /** Convert a Windows absolute path to a WSL /mnt/... path. */ | ||
| function toWslPath(winPath: string): string { | ||
| // e.g. C:\Users\foo → /mnt/c/Users/foo | ||
| return winPath | ||
| .replace(/^([A-Za-z]):\\/, (_, drive) => `/mnt/${drive.toLowerCase()}/`) | ||
| .replaceAll("\\", "/"); | ||
|
Comment on lines
+37
to
+42
There was a problem hiding this comment. 🧩 Analysis chain🏁 Script executed: # First, let's see the actual code around lines 37-50 in the file
cat -n benchmark/three-way-benchmark.ts | head -60 | tail -35Repository: codingstark-dev/bunkill Length of output: 1591 🏁 Script executed: # Search for where toWslPath is called and how dir parameter is passed
rg "toWslPath|npkillScan" benchmark/three-way-benchmark.ts -A 2 -B 2Repository: codingstark-dev/bunkill Length of output: 992 🏁 Script executed: # Find where SCAN_DIR is defined
rg "SCAN_DIR|--dir" benchmark/three-way-benchmark.ts -B 2 -A 2Repository: codingstark-dev/bunkill Length of output: 862 🏁 Script executed: # Verify the path resolution behavior by testing the proposed fix logic
python3 << 'EOF'
import re
# Current implementation
def toWslPath_current(winPath: str) -> str:
result = winPath
# Regex only matches backslashes
result = re.sub(r'^([A-Za-z]):\\', lambda m: f"/mnt/{m.group(1).lower()}/", result)
result = result.replace("\\", "/")
return result
# Proposed implementation (simulated on Unix)
def toWslPath_proposed(winPath: str) -> str:
# Simulate resolve() by converting mixed slashes to forward slashes
normalized = winPath.replace("\\", "/")
# Now match forward slash pattern
result = re.sub(r'^([A-Za-z]):/', lambda m: f"/mnt/{m.group(1).lower()}/", normalized)
return result
# Test cases
test_cases = [
".",
"./repo",
"C:\\Users\\foo",
"C:/Users/foo",
"C:\\Users\\foo's\\bar",
]
print("Testing path conversions:")
print("-" * 60)
for case in test_cases:
current = toWslPath_current(case)
proposed = toWslPath_proposed(case)
print(f"Input: {case!r}")
print(f" Current: {current!r}")
print(f" Proposed: {proposed!r}")
print()
EOFRepository: codingstark-dev/bunkill Length of output: 530 🏁 Script executed: # Also verify the shell escaping issue with a simple test
python3 << 'EOF'
def escapeForSingleQuotedShell(value: str) -> str:
return value.replace("'", "'\"'\"'")
# Test with a problematic path
path_with_quote = "/mnt/c/User's Files"
escaped = escapeForSingleQuotedShell(path_with_quote)
print("Shell escaping test:")
print(f"Original path: {path_with_quote!r}")
print(f"Escaped path: {escaped!r}")
print()
print("Bash command construction:")
print(f"Unescaped: bash -lc 'npkill -d \"{path_with_quote}\" -nu'")
print(f"Escaped: bash -lc 'npkill -d \"{escaped}\" -nu'")
EOFRepository: codingstark-dev/bunkill Length of output: 316 Resolve relative paths and escape shell metacharacters in WSL path conversion. The
Use Fix-import { join } from "node:path";
+import { join, resolve } from "node:path";
+
+function escapeForSingleQuotedShell(value: string): string {
+ return value.replaceAll("'", `'"'"'`);
+}
/** Convert a Windows absolute path to a WSL /mnt/... path. */
function toWslPath(winPath: string): string {
- // e.g. C:\Users\foo → /mnt/c/Users/foo
- return winPath
- .replace(/^([A-Za-z]):\\/, (_, drive) => `/mnt/${drive.toLowerCase()}/`)
- .replaceAll("\\", "/");
+ const normalized = resolve(winPath).replaceAll("\\", "/");
+ return normalized.replace(/^([A-Za-z]):\//, (_, drive) => `/mnt/${drive.toLowerCase()}/`);
}
async function npkillScan(dir: string, useWsl = false): Promise<number> {
const scriptCmd = useWsl
- ? ["wsl", "bash", "-lc", `script -q /dev/null -c 'npkill -d "${toWslPath(dir)}" -nu --hide-errors' 2>/dev/null`]
+ ? ["wsl", "bash", "-lc", `script -q /dev/null -c 'npkill -d "${escapeForSingleQuotedShell(toWslPath(dir))}" -nu --hide-errors' 2>/dev/null`]
: ["script", "-q", "/dev/null", "npkill", "-d", dir, "-nu", "--hide-errors"];🤖 Prompt for AI Agents |
||
| } | ||
|
|
||
| async function npkillScan(dir: string, useWsl = false): Promise<number> { | ||
| // Try to load Node environment in WSL by using a login shell. | ||
| // Add > /dev/null to discard TTY escape sequences that corrupt the Windows terminal. | ||
| const scriptCmd = useWsl | ||
| ? ["wsl", "bash", "-lc", `script -q /dev/null -c 'npkill -d "${toWslPath(dir)}" -nu --hide-errors' > /dev/null`] | ||
| : ["script", "-q", "/dev/null", "npkill", "-d", dir, "-nu", "--hide-errors"]; | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
|
|
||
| try { | ||
| const proc = Bun.spawn({ | ||
| cmd: scriptCmd, | ||
|
Comment on lines
+45
to
+55
There was a problem hiding this comment. 🧩 Analysis chain🏁 Script executed: cd benchmark && wc -l three-way-benchmark.tsRepository: codingstark-dev/bunkill Length of output: 93 🏁 Script executed: cd benchmark && sed -n '40,90p' three-way-benchmark.tsRepository: codingstark-dev/bunkill Length of output: 1697 🏁 Script executed: cd benchmark && sed -n '210,260p' three-way-benchmark.tsRepository: codingstark-dev/bunkill Length of output: 2058 🏁 Script executed: cd benchmark && sed -n '75,95p' three-way-benchmark.tsRepository: codingstark-dev/bunkill Length of output: 682 🏁 Script executed: cd benchmark && grep -n "exitCode\|exitStatus\|exit\|proc\." three-way-benchmark.ts | head -20Repository: codingstark-dev/bunkill Length of output: 381 🏁 Script executed: web_search
Bun.spawn exit code status checkingRepository: codingstark-dev/bunkill Length of output: 165 🌐 Web query:
💡 Result: To check the exit code of a process spawned with Bun.spawn, use the proc.exited Promise and the proc.exitCode property on the returned Subprocess object. Key methods: 1. Await proc.exited to wait for the process to exit, which resolves to the exit code: const proc = Bun.spawn(["bun", "--version"]); const exitCode = await proc.exited; // e.g. 0 for success console.log(exitCode); 2. After awaiting exited (or if already exited), read proc.exitCode synchronously (null if not exited): await proc.exited; console.log(proc.exitCode); // number | null 3. Use the onExit option in spawn options for a callback: const proc = Bun.spawn(["echo", "hello"], { onExit(subprocess, exitCode, signalCode, error) { console.log(Exited with code ${exitCode}); }, }); proc.exited is a Promise that resolves to the exit code when the process exits. Bun waits for all child processes to exit by default unless unref is called. This matches Node.js child_process behavior but uses ReadableStreams for stdio. Citations:
🏁 Script executed: cd benchmark && rg -A 5 -B 5 "proc\.exited" three-way-benchmark.tsRepository: codingstark-dev/bunkill Length of output: 420 🏁 Script executed: cd benchmark && grep -n "available" three-way-benchmark.ts | tail -10Repository: codingstark-dev/bunkill Length of output: 706 Check WSL tool availability before running benchmarks and inspect process exit codes.
Additionally, Probe inside WSL before enabling npkill (e.g., Also applies to: 218–224, 246–254, 79–87 🤖 Prompt for AI Agents |
||
| stdout: "pipe", | ||
| stderr: "pipe", | ||
| stdin: "pipe", | ||
|
|
@@ -84,11 +98,23 @@ async function bunGlobScan(dir: string): Promise<number> { | |
| onlyFiles: false, | ||
| followSymlinks: false, | ||
| })) { | ||
| // Normalize to forward slashes for consistent checks | ||
| const normalized = relative.replaceAll("\\", "/"); | ||
| const fullPath = join(dir, relative); | ||
| const fullNormalized = fullPath.replaceAll("\\", "/"); | ||
|
|
||
| // Skip nested node_modules (same as bunkill) | ||
| if (fullNormalized.includes("/node_modules/")) continue; | ||
| if (shouldSkip(fullNormalized)) continue; | ||
|
|
||
| const depth = normalized.split("/").filter(Boolean).length; | ||
| if (depth > MAX_DEPTH) continue; | ||
|
|
||
| // Require a package.json in the parent directory (same as bunkill) | ||
| const parentPath = fullPath.slice(0, fullPath.length - "/node_modules".length); | ||
| const pkgJson = Bun.file(join(parentPath, "package.json")); | ||
| if (!(await pkgJson.exists())) continue; | ||
|
Comment on lines
+114
to
+117
There was a problem hiding this comment.
|
||
|
|
||
| results.push(fullPath); | ||
| } | ||
|
|
||
|
|
@@ -162,19 +188,20 @@ function fmtMs(ms: number): string { | |
| } | ||
|
|
||
| function printRow(r: BenchResult, baseline: number): void { | ||
| // \r\x1b[2K clears any partial line left by WSL/npkill carriage returns | ||
| if (!r.available) { | ||
| process.stdout.write(`\r\x1b[2K ${r.label.padEnd(24)} N/A (${r.note ?? "unavailable"})\n`); | ||
| return; | ||
| } | ||
|
|
||
| const speedup = r.avgMs === baseline | ||
| ? "baseline" | ||
| : `${(baseline / r.avgMs).toFixed(1)}x faster`; | ||
|
|
||
| process.stdout.write( | ||
| `\r\x1b[2K ${r.label.padEnd(24)} avg=${fmtMs(r.avgMs).padStart(8)} ` + | ||
| `min=${fmtMs(r.minMs).padStart(8)} max=${fmtMs(r.maxMs).padStart(8)} ` + | ||
| `found=${String(r.found).padStart(4)} ${speedup}\n`, | ||
| ); | ||
| } | ||
|
|
||
|
|
@@ -184,9 +211,27 @@ console.log("╚═════════════════════ | |
| console.log(` Scan root : ${SCAN_DIR}`); | ||
| console.log(` Runs : ${RUNS} (+ 1 warm-up)\n`); | ||
|
|
||
| const isWindows = process.platform === "win32"; | ||
|
|
||
| // Detect npkill | ||
| const npkillPath = isWindows | ||
| ? await Bun.$.nothrow()`where npkill`.text().then((s) => s.trim().split("\n")[0]?.trim() ?? "").catch(() => "") | ||
| : await Bun.$.nothrow()`which npkill`.text().then((s) => s.trim()).catch(() => ""); | ||
|
|
||
| // Detect WSL (Windows only) | ||
| const wslAvailable = isWindows | ||
| ? await Bun.$.nothrow()`where wsl`.text().then((s) => s.trim().length > 0).catch(() => false) | ||
| : false; | ||
|
|
||
| const npkillStatus = !npkillPath | ||
| ? "NOT FOUND (npm install -g npkill)" | ||
| : isWindows && wslAvailable | ||
| ? `${npkillPath} (via WSL)` | ||
| : isWindows | ||
| ? `${npkillPath} (skipped: WSL not found — install WSL to enable)` | ||
| : npkillPath; | ||
|
|
||
| console.log(` npkill : ${npkillStatus}`); | ||
| console.log(` Bun.Glob : baseline glob walk`); | ||
| console.log(` bunkill : current scanner.ts implementation\n`); | ||
|
|
||
|
|
@@ -197,9 +242,15 @@ const [globResult, bunkillResult] = await Promise.all([ | |
| bench("bunkill current", bunKillScan, SCAN_DIR, RUNS), | ||
| ]); | ||
|
|
||
| // On Windows: run npkill via WSL if available; on Unix: run directly | ||
| const canRunNpkill = npkillPath && (!isWindows || wslAvailable); | ||
| const npkillResult = canRunNpkill | ||
| ? await bench("npkill", (d) => npkillScan(d, isWindows && wslAvailable), SCAN_DIR, 1) | ||
| : { | ||
| label: "npkill", | ||
| avgMs: 0, minMs: 0, maxMs: 0, found: -1, available: false, | ||
| note: !npkillPath ? "not installed" : "WSL not found (install WSL to enable)", | ||
| }; | ||
|
|
||
| const available = [npkillResult, globResult, bunkillResult].filter((r) => r.available); | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -5,7 +5,7 @@ import { stat } from "node:fs/promises"; | |||||||||
| import { basename, join, resolve } from "node:path"; | ||||||||||
| import { filesize } from "filesize"; | ||||||||||
| import { APP_CONFIG } from "./config.ts"; | ||||||||||
| import { deleteModules, normalizeProjectPath, scan as scanEngine } from "./scanner.ts"; | ||||||||||
| import type { NodeModule, ScanOptions } from "./types.ts"; | ||||||||||
|
|
||||||||||
| const LOGO = ` | ||||||||||
|
|
@@ -558,7 +558,7 @@ class BunKill { | |||||||||
| } | ||||||||||
|
|
||||||||||
| this.pendingUiMeta.add(module.path); | ||||||||||
| const projectPath = normalizeProjectPath(module.path); | ||||||||||
|
There was a problem hiding this comment.
Suggested change
|
||||||||||
|
|
||||||||||
| try { | ||||||||||
| const result = await Bun.$`git -C ${projectPath} status --short --branch`.quiet().nothrow(); | ||||||||||
|
|
||||||||||
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.
CI uses
bun-version: latest, which can make builds non-deterministic and introduce sudden breakages when Bun releases. Consider pinning to a specific Bun version (or at least the minimum supported in package.json engines) and optionally adding a separate scheduled job forlatestif you want early warning.