Add completion install, uninstall, and status subcommands#4581
Open
simonfaltum wants to merge 15 commits intomainfrom
Open
Add completion install, uninstall, and status subcommands#4581simonfaltum wants to merge 15 commits intomainfrom
simonfaltum wants to merge 15 commits intomainfrom
Conversation
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.
Collaborator
|
Commit: 5028c89
15 interesting tests: 7 KNOWN, 7 SKIP, 1 RECOVERED
Top 20 slowest tests (at least 2 minutes):
|
- 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.
…lank-line collapse, Sprintf padding
andrewnester
approved these changes
Feb 24, 2026
Contributor
andrewnester
left a comment
There was a problem hiding this comment.
Could you please add an entry to NEXT_CHANGELOG.md before merging the PR?
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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. Addinginstall/uninstall/statussubcommands automates the setup.Changes
Adds three new subcommands to the existing
completioncommand:completion install [--shell <shell>] [--auto-approve]— Auto-detects shell from$SHELL, appends an eval shim wrapped inBEGIN/ENDmarkers 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-approveskips 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
$SHELLfirst (catches Git Bash/MSYS on Windows), falls back to PATH lookup on Windows. If$SHELLcontains 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 CobraOutOrStdout()capture timing issue with the test harness.Tests
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.exefallback)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