[FEATURE] Enable zsh integration #107
Conversation
|
/cc @paccorsi |
pkg/integration/zsh.go
Outdated
| package integration | ||
|
|
||
| var zshSource = ` | ||
| prmptcmd() { $("__dad_prompt_command") } |
There was a problem hiding this comment.
Do we need to call the prompt command in a subshell with $(...) ?
Could we just call it directly? prmptcmd() { __dad_prompt_command }
There was a problem hiding this comment.
Yes sadly it seems it needs to be called in a subshell cause if not, it would run as soon as you type prmptcmd() { __dad_prompt_command } and not everytime a prompt is being displayed
pkg/integration/integration.go
Outdated
| fmt.Println(bashSource) | ||
| var currentShell = os.Getenv("SHELL") | ||
|
|
||
| fmt.Println(shellSource) |
There was a problem hiding this comment.
Should we print most of the source already if we have a major failure condition right after?
If we are running an unsupported shell, I think we should not print anything to stdout, because it will be eval'ed... by this unsupported shell.
I know I mentioned the general idea of preferring warnings rather than stopping execution...(because it could partially work and that could be enough for some people) but in this case, it looks like it could actually be very dangerous...
There was a problem hiding this comment.
Yeah you're right it should work without shell integration but we should not have a half one
pkg/integration/integration.go
Outdated
| } else if strings.HasSuffix(currentShell, "zsh") { | ||
| fmt.Println(zshSource) | ||
| } else { | ||
| fmt.Println(color.Brown("Your shell is not supported")) |
There was a problem hiding this comment.
The execution context is an eval:
eval "$(dad --shell-init --with-completion)"So anything printed on the stdout will be eval'ed.
To display a warning, it must be printed on stderr.
|
|
||
| color "github.com/logrusorgru/aurora" | ||
| ) | ||
|
|
||
| func Print() { | ||
| fmt.Println(bashSource) | ||
| var currentShell = os.Getenv("SHELL") |
There was a problem hiding this comment.
Just an idea, we could warn in case the value is empty, since it would be a very special case of "unknown shell".
That may help someone debug their environment someday.
pkg/integration/integration.go
Outdated
| } else if strings.HasSuffix(currentShell, "zsh") { | ||
| fmt.Println(shellSource, zshSource) | ||
| } else if { | ||
| fmt.Fprintln(os.Stderr, color.Red("Your shell configuration is undefined")) |
There was a problem hiding this comment.
👍 Maybe mention that the SHELL variable itself is empty, and this is what's weird.
pkg/integration/integration.go
Outdated
| } else { | ||
| fmt.Fprintln(os.Stderr, color.Brown("Your shell is not supported")) | ||
| } | ||
| return nil |
pkg/integration/integration.go
Outdated
| fmt.Println(shellSource, bashSource) | ||
| } else if strings.HasSuffix(currentShell, "zsh") { | ||
| fmt.Println(shellSource, zshSource) | ||
| } else if { |
pkg/integration/zsh.go
Outdated
|
|
||
| var zshSource = ` | ||
| prmptcmd() { $("__dad_prompt_command") } | ||
| precmd_functions=(prmptcmd) |
There was a problem hiding this comment.
precmd_functions+=(prmptcmd)
To preserve the other pre-command hooks.
There was a problem hiding this comment.
It could also be wrapped in a if statement in case dad is already activated
if [[ ! "${precmd_functions[@]}" == *__dad_prompt_command* ]]; then
precmd_functions+=(prmptcmd)
fi
dev.yml
Outdated
| @@ -1,4 +1,5 @@ | |||
| up: | |||
| - python: 3.6.5 | |||
Why
This PR is enable shell integration for zsh
How
Splitting out the logic of the shell integration into separated file: 1 for bash, 1 for zsh and a common 1
The approach taken was inspired by https://superuser.com/questions/735660/whats-the-zsh-equivalent-of-bashs-prompt-command/735969#735969
Example: