Skip to content

Comments

Add completion install, uninstall, and status subcommands#4581

Open
simonfaltum wants to merge 15 commits intomainfrom
simonfaltum/completion-install-uninstall
Open

Add completion install, uninstall, and status subcommands#4581
simonfaltum wants to merge 15 commits intomainfrom
simonfaltum/completion-install-uninstall

Conversation

@simonfaltum
Copy link
Member

@simonfaltum simonfaltum commented Feb 23, 2026

Why

Users currently have to manually configure shell tab completion by piping databricks completion <shell> output into the right RC file. This is error-prone and shell-specific. Adding install/uninstall/status subcommands automates the setup.

Changes

Adds three new subcommands to the existing completion command:

databricks completion
  databricks completion bash          # Generate bash completion script
  databricks completion zsh           # Generate zsh completion script
  databricks completion fish          # Generate fish completion script
  databricks completion powershell    # Generate powershell completion script
  databricks completion install       # NEW: Install completions into shell RC file
  databricks completion uninstall     # NEW: Remove completions from shell RC file
  databricks completion status        # NEW: Show current completion status

completion install [--shell <shell>] [--auto-approve] — Auto-detects shell from $SHELL, appends an eval shim wrapped in BEGIN/END markers to the appropriate RC file. Fish uses a file drop to ~/.config/fish/completions/ instead. Shows the detected shell and target file and asks for confirmation before writing; --auto-approve skips the prompt (for scripts/agents). If completions are already present (our marker, Homebrew, or an external fish file), reports that and exits without prompting.

completion uninstall [--shell <shell>] [--auto-approve] — Removes the marker block from the RC file (or deletes the fish completions file if it contains our marker). Strict parsing: returns an error if markers are corrupted rather than risking RC file damage. External completions (Homebrew, package manager fish files) are detected and reported but not removed.

completion status [--shell <shell>] — Reports whether completions are installed, detecting our marker block, Homebrew-based zsh installs, and fish file-based installs. Also serves as a dry-run to preview the detected shell and target file before running install.

Shell support: bash, zsh, fish, powershell (pwsh 7+), powershell5 (Windows PowerShell 5.1). Shell detection checks $SHELL first (catches Git Bash/MSYS on Windows), falls back to PATH lookup on Windows. If $SHELL contains an unrecognized value on Windows (e.g. powershell.exe), detection falls through to PATH-based lookup.

Architecture: Core logic lives in libs/completion/ with no cobra dependency, so it can be called from other commands (e.g. a guided setup flow). cmd/completion/ contains thin cobra wrappers. The existing shell generation subcommands (bash/zsh/fish/powershell) are reimplemented with runtime writer resolution to avoid a Cobra OutOrStdout() capture timing issue with the test harness.

Tests

  • Unit tests in libs/completion/ covering shell detection, install/uninstall/status logic, idempotency, marker corruption detection, edge cases (empty files, missing dirs, permission preservation, Windows permission semantics, fish foreign-file ownership, $SHELL=powershell.exe fallback)
  • Acceptance test in acceptance/cmd/completion/ covering the full install → status → idempotent reinstall → uninstall → status round-trip, plus smoke tests for all four shell script generators and --no-descriptions

Add `databricks completion install`, `completion uninstall`, and
`completion status` commands that auto-detect the user's shell and
manage tab completion configuration.

Install appends an eval shim (wrapped in BEGIN/END markers) to the
appropriate RC file. Uninstall removes it. Status reports whether
completions are installed, including detection of Homebrew-based
installs for zsh.

Supported shells: bash, zsh, fish, powershell (pwsh 7+),
powershell5 (Windows PowerShell 5.1).

The core logic lives in libs/completion/ for reusability from other
commands (e.g. guided setup flows). The cmd/completion/ package
provides thin cobra wrappers. Shell script generation subcommands
(bash/zsh/fish/powershell) are reimplemented with runtime writer
resolution to avoid a Cobra output-capture timing issue.
@eng-dev-ecosystem-bot
Copy link
Collaborator

eng-dev-ecosystem-bot commented Feb 23, 2026

Commit: 5028c89

Run: 22353900393

Env 🟨​KNOWN 💚​RECOVERED 🙈​SKIP ✅​pass 🙈​skip Time
🟨​ aws linux 7 1 7 268 764 7:13
🟨​ aws windows 7 1 7 270 762 7:28
💚​ aws-ucws linux 8 7 364 680 8:09
💚​ aws-ucws windows 8 7 366 678 6:24
💚​ azure linux 2 9 271 762 5:37
💚​ azure windows 2 9 273 760 5:57
💚​ azure-ucws linux 2 9 369 676 8:35
💚​ azure-ucws windows 2 9 371 674 6:52
💚​ gcp linux 2 9 267 765 6:01
💚​ gcp windows 2 9 269 763 5:10
15 interesting tests: 7 KNOWN, 7 SKIP, 1 RECOVERED
Test Name aws linux aws windows aws-ucws linux aws-ucws windows azure linux azure windows azure-ucws linux azure-ucws windows gcp linux gcp windows
🟨​ TestAccept 🟨​K 🟨​K 💚​R 💚​R 💚​R 💚​R 💚​R 💚​R 💚​R 💚​R
🙈​ TestAccept/bundle/resources/permissions 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S
🟨​ TestAccept/bundle/resources/permissions/jobs/destroy_without_mgmtperms/with_permissions 🟨​K 🟨​K 💚​R 💚​R 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S
🟨​ TestAccept/bundle/resources/permissions/jobs/destroy_without_mgmtperms/with_permissions/DATABRICKS_BUNDLE_ENGINE=direct 🟨​K 🟨​K 💚​R 💚​R
🟨​ TestAccept/bundle/resources/permissions/jobs/destroy_without_mgmtperms/with_permissions/DATABRICKS_BUNDLE_ENGINE=terraform 🟨​K 🟨​K 💚​R 💚​R
🟨​ TestAccept/bundle/resources/permissions/jobs/destroy_without_mgmtperms/without_permissions 🟨​K 🟨​K 💚​R 💚​R 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S
🟨​ TestAccept/bundle/resources/permissions/jobs/destroy_without_mgmtperms/without_permissions/DATABRICKS_BUNDLE_ENGINE=direct 🟨​K 🟨​K 💚​R 💚​R
🟨​ TestAccept/bundle/resources/permissions/jobs/destroy_without_mgmtperms/without_permissions/DATABRICKS_BUNDLE_ENGINE=terraform 🟨​K 🟨​K 💚​R 💚​R
🙈​ TestAccept/bundle/resources/postgres_branches/basic 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S
🙈​ TestAccept/bundle/resources/postgres_branches/recreate 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S
🙈​ TestAccept/bundle/resources/postgres_branches/update_protected 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S
🙈​ TestAccept/bundle/resources/postgres_branches/without_branch_id 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S
🙈​ TestAccept/bundle/resources/postgres_endpoints/recreate 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S
🙈​ TestAccept/bundle/resources/synced_database_tables/basic 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S 🙈​S
💚​ TestAccept/ssh/connection 💚​R 💚​R 💚​R 💚​R 💚​R 💚​R 💚​R 💚​R 💚​R 💚​R
Top 20 slowest tests (at least 2 minutes):
duration env testname
4:28 gcp linux TestAccept/bundle/resources/apps/inline_config/DATABRICKS_BUNDLE_ENGINE=terraform
4:15 azure windows TestAccept/bundle/resources/apps/inline_config/DATABRICKS_BUNDLE_ENGINE=direct
3:55 azure-ucws windows TestAccept/bundle/resources/apps/inline_config/DATABRICKS_BUNDLE_ENGINE=terraform
3:49 gcp windows TestAccept/bundle/resources/apps/inline_config/DATABRICKS_BUNDLE_ENGINE=terraform
3:40 aws-ucws linux TestAccept/bundle/resources/apps/inline_config/DATABRICKS_BUNDLE_ENGINE=terraform
3:37 gcp windows TestAccept/bundle/resources/apps/inline_config/DATABRICKS_BUNDLE_ENGINE=direct
3:31 aws windows TestAccept/bundle/resources/apps/inline_config/DATABRICKS_BUNDLE_ENGINE=direct
3:29 aws-ucws linux TestAccept/bundle/resources/apps/inline_config/DATABRICKS_BUNDLE_ENGINE=direct
3:17 aws-ucws windows TestAccept/bundle/resources/apps/inline_config/DATABRICKS_BUNDLE_ENGINE=direct
3:08 aws-ucws windows TestAccept/bundle/resources/apps/inline_config/DATABRICKS_BUNDLE_ENGINE=terraform
3:08 gcp linux TestAccept/bundle/resources/apps/inline_config/DATABRICKS_BUNDLE_ENGINE=direct
3:02 aws windows TestAccept/bundle/resources/apps/inline_config/DATABRICKS_BUNDLE_ENGINE=terraform
2:55 aws linux TestAccept/bundle/resources/apps/inline_config/DATABRICKS_BUNDLE_ENGINE=terraform
2:52 aws linux TestAccept/bundle/resources/apps/inline_config/DATABRICKS_BUNDLE_ENGINE=direct
2:20 azure-ucws linux TestAccept/bundle/resources/apps/inline_config/DATABRICKS_BUNDLE_ENGINE=terraform
2:16 azure linux TestAccept/bundle/resources/apps/inline_config/DATABRICKS_BUNDLE_ENGINE=terraform
2:13 azure-ucws windows TestAccept/bundle/resources/apps/inline_config/DATABRICKS_BUNDLE_ENGINE=direct
2:10 azure windows TestAccept/bundle/resources/apps/inline_config/DATABRICKS_BUNDLE_ENGINE=terraform
2:10 azure-ucws linux TestAccept/bundle/resources/apps/inline_config/DATABRICKS_BUNDLE_ENGINE=direct
2:10 azure linux TestAccept/bundle/resources/apps/inline_config/DATABRICKS_BUNDLE_ENGINE=direct

- Fix uninstall removing the trailing newline of the line before the
  marker block, which could join adjacent lines
- Fix acceptance test using unbound $TMPDIR on Linux CI; use mktemp -d
- Use zsh in acceptance test to avoid OS-dependent bash RC file path
- Suppress Homebrew detection in test with HOMEBREW_PREFIX=/nonexistent
- Fix install message suggesting 'source' for PowerShell shells
- Tighten uninstall tests to assert exact file content after removal
- Remove unused supportedShells variable (lint)
Windows does not honor Unix permission bits, so the 0o600 written
mode reads back as 0o666. Guard the assertion with runtime.GOOS and
fall back to a write-ability check on Windows.
Two correctness fixes:
- DetectShell: if $SHELL is set but unrecognized on Windows (e.g.
  powershell.exe), fall through to detectWindowsShell() instead of
  returning "unsupported shell". On non-Windows the error is preserved.
- uninstallFish: only delete the completions file if it contains our
  BeginMarker. Foreign files (e.g. installed by a package manager) are
  left untouched and wasInstalled=false is returned.

Tests: add TestDetectShellPowershellExeNonWindows, update TestUninstallFish
to use CLI-managed content, add TestUninstallFishForeignFile.
Handle PowerShell-style SHELL values without masking unsupported shells, and avoid overwriting externally managed fish completion files while improving uninstall messaging.

Co-authored-by: Cursor <cursoragent@cursor.com>
Use the shared sethome helper in acceptance tests and normalize displayed file paths so output remains stable across platforms and avoids HOME/UserHomeDir mismatches.

Co-authored-by: Cursor <cursoragent@cursor.com>
Before modifying shell config files, show the detected shell and target
file path and ask the user to confirm. This defends against heuristic
misdetection. The prompt is skipped when --yes is passed or when the
operation is a no-op (already installed / not installed).

In non-interactive mode without --yes, the command returns an error
hinting at the flag, following the same pattern as bundle deploy.
Align with existing CLI convention (bundle deploy/destroy use
--auto-approve). The non-interactive error message now suggests
'databricks completion status' as a dry-run to preview the detected
shell and target file.
Three fixes from review:

- Install command: early-return for any result.Installed (not just
  "marker"). Fish external files and Homebrew completions now get
  appropriate messages without a useless prompt. The alreadyInstalled
  return from Install() is used as a safety net for final messaging.

- Uninstall command: homebrew message no longer shows the misleading
  RC file path. Fish external files get "installed externally in <path>".

- installFish: both file-exists branches returned the same value;
  simplify to a single os.Stat check.
Without compinit, neither our eval shim nor Homebrew's _databricks
file will be loaded by zsh. Add a shared warnIfCompinitMissing helper
that checks for "compinit" in .zshrc and prints actionable guidance.

The warning appears on install (after success or already-installed),
status (when installed), and uninstall (when completions remain via
an external method like Homebrew).
The eval shim in .zshrc and Homebrew's _databricks in site-functions
are separate files that coexist. Don't block explicit install when the
user has Homebrew completions — print an informational note and
proceed with the normal prompt/install flow.

External fish files (no marker) still early-return since installFish
cannot overwrite them.
@simonfaltum simonfaltum marked this pull request as ready for review February 24, 2026 08:10
Copy link
Contributor

@andrewnester andrewnester left a comment

Choose a reason for hiding this comment

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

Could you please add an entry to NEXT_CHANGELOG.md before merging the PR?

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.

3 participants