Skip to content

fix: completions on git bash for windows#28122

Open
Zamiell wants to merge 6 commits intooven-sh:mainfrom
Zamiell:fix-completions-on-git-bash
Open

fix: completions on git bash for windows#28122
Zamiell wants to merge 6 commits intooven-sh:mainfrom
Zamiell:fix-completions-on-git-bash

Conversation

@Zamiell
Copy link

@Zamiell Zamiell commented Mar 14, 2026

What does this PR do?

Fixes #25662.

How did you verify your code works?

Proof that the PR works:

james@1qaz MINGW64 /c/Repositories/bun (fix-completions-on-git-bash)
$ bun completions
error: PowerShell completions are not yet written for Bun yet.
See https://github.com/oven-sh/bun/issues/8939

james@1qaz MINGW64 /c/Repositories/bun (fix-completions-on-git-bash)
$ bun bd
$ BUN_DEBUG_QUIET_LOGS=1 ./scripts/bd
Bun is a fast JavaScript runtime, package manager, bundler, and test runner. (1.3.11-debug+31f0bd63e)

Usage: bun <command> [...flags] [...args]

Commands:
  run       ./my-script.ts       Execute a file with Bun
            lint                 Run a package.json script
  test                           Run unit tests with Bun
  x         vite                 Execute a package binary (CLI), installing if needed (bunx)
  repl                           Start a REPL session with Bun
  exec                           Run a shell script directly with Bun

  install                        Install dependencies for a package.json (bun i)
  add       @zarfjs/zarf         Add a dependency to package.json (bun a)
  remove    webpack              Remove a dependency from package.json (bun rm)
  update    zod                  Update outdated dependencies
  audit                          Check installed packages for vulnerabilities
  outdated                       Display latest versions of outdated dependencies
  link      [<package>]          Register or link a local npm package
  unlink                         Unregister a local npm package
  publish                        Publish a package to the npm registry
  patch <pkg>                    Prepare a package for patching
  pm <subcommand>                Additional package management utilities
  info      tailwindcss          Display package metadata from the registry
  why       elysia               Explain why a package is installed

  build     ./a.ts ./b.jsx       Bundle TypeScript & JavaScript into a single file

  init                           Start an empty Bun project from a built-in template
  create    astro                Create a new project from a template (bun c)
  upgrade                        Upgrade to latest version of Bun.
  feedback  ./file1 ./file2      Provide feedback to the Bun team.

  <command> --help               Print help text for command.

Learn more about Bun:            https://bun.com/docs
Join our Discord community:      https://bun.com/discord

james@1qaz MINGW64 /c/Repositories/bun (fix-completions-on-git-bash)
$ ./build/debug/bun-debug completions
[sys] CreateHardLinkW(\??\C:\Repositories\bun\build\debug\bunx-debug.exe, C:\Repositories\bun\build\debug\bun-debug.exe) = 0
error: Could not find a directory to install completions in.
Please either pipe it:
   bun completions > /to/a/file

 Or pass a directory:

   bun completions /my/completions/dir

james@1qaz MINGW64 /c/Repositories/bun (fix-completions-on-git-bash)
$

@Zamiell Zamiell changed the title fix: git bash on windows fix: completions on git bash for windows Mar 14, 2026
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Mar 14, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 88061e0d-9ce2-4ee8-823f-ec6c7fa104e9

📥 Commits

Reviewing files that changed from the base of the PR and between 538b965 and c6c9bba.

📒 Files selected for processing (1)
  • test/regression/issue/25042.test.ts

Walkthrough

Adds Windows/MSYS path conversion for tar extraction with relaxed symlink error handling on Windows; moves PowerShell completions into the shell switch; normalizes SHELL detection across platforms; exposes a Windows key for the SHELL env var; and adds Windows-targeted regression tests for completions.

Changes

Cohort / File(s) Summary
Build & Path Handling
scripts/build/download.ts
Add msysPath to convert MSYS/Git Bash paths to POSIX; apply it to tarball and destination before extraction; on Windows treat tar failures that are only symlink-related as warnings, otherwise preserve error behavior.
CLI Completions Command
src/cli/install_completions_command.zig
Move PowerShell completions handling into the shell switch as an explicit .pwsh case; replace fpath.get() with fpath.platformGet() for zsh fpath lookup.
Shell Detection
src/cli/shell_completions.zig
Normalize SHELL by extracting the basename from paths with '/' or '\' and stripping a trailing .exe, preserving mapping to known shells (.bash/.zsh/.fish/.pwsh/.unknown).
Environment Variable Keys
src/env_var.zig
Change public SHELL declaration to provide a Windows key as well ("SHELL", "SHELL"), enabling platform-specific retrieval on Windows.
Regression Tests
test/regression/issue/25042.test.ts
Add Windows-gated tests validating completions behavior for /usr/bin/bash (Git Bash/MSYS) and pwsh, covering TTY vs non-TTY output, stderr content, and exit codes.
🚥 Pre-merge checks | ✅ 2 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Linked Issues check ⚠️ Warning The PR claims to fix issue #25662, but the code changes address Git Bash completions on Windows, not the website tab detection issue that #25662 describes. Verify the correct issue number or update PR description to reference the actual Git Bash completions issue if a different linked issue exists.
Out of Scope Changes check ⚠️ Warning Code changes focus on shell completions for Git Bash on Windows (scripts/build/download.ts, src/cli/, src/env_var.zig, test/), which are entirely unrelated to the website tab detection requirement in issue #25662. Clarify the relationship between the PR and linked issue #25662, or link to the correct issue for Git Bash completions fixes.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title 'fix: completions on git bash for windows' directly and clearly summarizes the main change: resolving completions functionality on Git Bash for Windows environments.
Description check ✅ Passed The description follows the template with both required sections completed: 'What does this PR do?' references issue #25662, and 'How did you verify your code works?' includes detailed terminal transcripts demonstrating the fix on Git Bash/Windows.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

📝 Coding Plan
  • Generate coding plan for human review comments

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@scripts/build/download.ts`:
- Around line 121-143: When only symlink extraction failures are detected
(inside the if block where process.platform === "win32" and fatalLines.length
=== 0), emit a warning before continuing: log a clear message that tar
extraction had symlink-only failures including the tarball identifier (tarball)
and the filtered stderr lines (fatalLines or result.stderr) to aid debugging;
use the project's logger if available (e.g., processLogger.warn) or console.warn
as a fallback, and do not change the existing BuildError throw behavior for real
failures.

In `@test/regression/issue/25042.test.ts`:
- Around line 29-45: In the test case inside the test(...) block that calls
spawnSync (variables stdout, stderr, exitCode), move the assertion that inspects
stdout (the expect on stdoutText.toContain("bun")) to occur before the
expect(exitCode).toBe(0) so the test checks output content prior to exit status;
update the sequence around the lines using stdout.toString() and the two expect
calls accordingly in the same test function.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: c99367ae-5c2c-495a-966b-e93e64524c65

📥 Commits

Reviewing files that changed from the base of the PR and between 10bdb48 and 31f0bd6.

📒 Files selected for processing (5)
  • scripts/build/download.ts
  • src/cli/install_completions_command.zig
  • src/cli/shell_completions.zig
  • src/env_var.zig
  • test/regression/issue/25042.test.ts

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@test/regression/issue/25042.test.ts`:
- Around line 8-27: The test "does not show PowerShell error when SHELL is bash"
currently only checks non-zero exitCode branch but doesn't assert that exitCode
is one of the expected values; update the assertion to explicitly validate
exitCode is either 0 or 1 (e.g. assert [0,1] contains exitCode or two explicit
expects) using the existing exitCode variable, and keep the existing stderrText
PowerShell checks inside the non-zero branch so unrelated failures don't pass.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: a511aaf3-fa87-4a07-8010-be556793985e

📥 Commits

Reviewing files that changed from the base of the PR and between 31f0bd6 and 538b965.

📒 Files selected for processing (2)
  • scripts/build/download.ts
  • test/regression/issue/25042.test.ts

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Website should default to Windows tab when using Windows

1 participant