Fix CLI completions install on silicon macs#507
Conversation
pkg/cmd/completion.go
Outdated
| if utils.IsMacOS() { | ||
| homeDir, err := os.UserHomeDir() | ||
| if err != nil { | ||
| utils.HandleError(err, "Unable to determine home directory") | ||
| } | ||
| // Check if oh-my-zsh is installed - it auto-adds ~/.oh-my-zsh/completions to fpath | ||
| omzDir := fmt.Sprintf("%s/.oh-my-zsh", homeDir) | ||
| if utils.Exists(omzDir) { | ||
| path = fmt.Sprintf("%s/.oh-my-zsh/completions", homeDir) | ||
| usingOhMyZsh = true | ||
| } else { | ||
| // Fall back to ~/.zsh/completions for non-oh-my-zsh users | ||
| path = fmt.Sprintf("%s/.zsh/completions", homeDir) | ||
| } | ||
| } else { | ||
| path = "/usr/local/share/zsh/site-functions" | ||
| } |
There was a problem hiding this comment.
nit: I'd really prefer for this to be OS-agnostic, and not have any special handling for OMZ (since it really doesn't matter). It's not guaranteed for /usr/local/share/ to be writable on other OSes either so we need to gracefully handle when it's not.
pkg/cmd/completion.go
Outdated
| } | ||
|
|
||
| // Write the fpath configuration block | ||
| configBlock := fmt.Sprintf("\n# Doppler CLI completions\n%s\nautoload -Uz compinit && compinit\n", fpathLine) |
There was a problem hiding this comment.
blocking: I don't trust that this will be reliable across all possible environments. Best case, we slow down every user's shell startup time by calling compinit unnecessarily, in others cases we might actually break things by doing this.
I'd much rather just print the line that needs to be added to the user's file in the case that it needs to be added manually, and expect the user to add it to the correct place.
pkg/cmd/completion.go
Outdated
| usingOhMyZsh = true | ||
| } else { | ||
| // Fall back to ~/.zsh/completions for non-oh-my-zsh users | ||
| path = fmt.Sprintf("%s/.zsh/completions", homeDir) |
There was a problem hiding this comment.
nit: I would not expect another program to write to this directory unless it were very well know and standardized, which is not the case here. We should be writing to our own directory.
There was a problem hiding this comment.
@emily-curry just to clarify, we do write it to a file named "_doppler", so everything gets written to "~/.zsh/completions/_doppler". Do we want to write this to a new directory entirely or does the filename suffice?
amoses12
force-pushed
the
austin/mac-completions-install
branch
from
d2fe889 to
4bc777a
Compare
amoses12
force-pushed
the
austin/mac-completions-install
branch
from
4bc777a to
b64ba0c
Compare
There was a problem hiding this comment.
nit: This file isn't added to tests/e2e.sh, so it won't be run in CI.
amoses12
force-pushed
the
austin/mac-completions-install
branch
from
b64ba0c to
bcf7f08
Compare
3 participants
Release Note: Fix CLI completions installation on silicon Macs.
Closes ENG-8130