Skip to content

Commit

Permalink
added completion for fish,zsh,bash, pwsh
Browse files Browse the repository at this point in the history
  • Loading branch information
@edeediong
edeediong committed Oct 10, 2023
1 parent 70e8740 commit 0e03d61
Showing 1 changed file with 42 additions and 9 deletions.
51 changes: 42 additions & 9 deletions src/runme/install.sh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,16 +25,49 @@ $nanolayer_location \

# Need a new shell to pick up NVM paths
bash -i << EOF
if [ "$COMPLETIONS" = "true" ]; then {
# runme bash completion
mkdir -p /etc/bash_completion.d
runme completion bash > /etc/bash_completion.d/runme
# runme zsh completion
if [ -e "/usr/share/zsh/vendor-completions" ]; then
runme completion zsh > /usr/share/zsh/vendor-completions/_runme
if [ "$COMPLETIONS" = "true" ]; then
# Check if the current shell is Bash
if [ "$(basename $SHELL)" = "bash" ]; then
# runme bash completion
mkdir -p /etc/bash_completion.d
runme completion bash > /etc/bash_completion.d/runme
source /etc/bash_completion.d/runme
fi
} fi
# Check if the current shell is Zsh
if [ "$(basename $SHELL)" = "zsh" ]; then
# runme zsh completion
ZSH_COMPLETIONS_DIR="/usr/share/zsh/site-functions/"
mkdir -p $ZSH_COMPLETIONS_DIR
runme completion zsh > "$ZSH_COMPLETIONS_DIR/_runme"
source $ZSH_COMPLETIONS_DIR/_runme
fi
# Check if the current shell is fish
if [ "$(basename $SHELL)" = "fish" ]; then
# runme fish completion
FISH_COMPLETIONS_DIR="/usr/share/fish/vendor_completions.d"
mkdir -p $FISH_COMPLETIONS_DIR
runme completion fish > "$FISH_COMPLETIONS_DIR/_runme"
source $FISH_COMPLETIONS_DIR/_runme
fi
# Check if the current shell is PowerShell
if [ "$(basename $SHELL)" = "pwsh" ]; then
# Enable Powershell tab-completion
Set-PSReadlineKeyHandler -Key Tab -Function Complete
mkdir -p $FISH_COMPLETIONS_DIR
runme completion fish > "$FISH_COMPLETIONS_DIR/_runme"
source $FISH_COMPLETIONS_DIR/_runme
fi
fi
EOF

echo 'Done!'

3 comments on commit 0e03d61

@CodeMan99
Copy link
Contributor

@CodeMan99 CodeMan99 commented on 0e03d61 Oct 18, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

@edeediong The install.sh script is executed as a docker build step. Line #27 also creates a new "interactive" bash shell.

The conditionals to check current shell will always be bash. The build system user is typically root. The development shell is very likely to be something other than bash.

@edeediong
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Hi @CodeMan99 thank you for the feedback.. I have removed the new interactive bash shell. Let me know if you come across another issue.

@CodeMan99
Copy link
Contributor

@CodeMan99 CodeMan99 commented on 0e03d61 Oct 24, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

@edeediong Removing the interactive bash invocation is not what I expected at all. I meant that it is invalid to check the current shell.

  1. The script itself is bash.
  2. It is executed by root as a docker image build step.
  3. The user can't run the script directly.
  4. The user is very likely to be using a different system user like vscode instead of root.
  5. The user's development shell is likely to be different than the build shell.

Additionally, removing the bash -i has broken the build step.

$ devcontainer features test
...
82.40 nvm cache cleared.
82.44 Done!

87.89 added 79 packages in 5s
87.89 
87.89 11 packages are looking for funding
87.89   run `npm fund` for details
87.89 npm notice 
87.89 npm notice New minor version of npm available! 10.1.0 -> 10.2.1
87.89 npm notice Changelog: https://github.com/npm/cli/releases/tag/v10.2.1
87.89 npm notice Run npm install -g npm@10.2.1 to update!
87.89 npm notice 
87.89 bash: runme: command not found
...

Notice the last line I've included here, it's saying that it can't find the runme executable. The new shell solved that by updating the $PATH.

Here's a gist of the full test log, which was executed at 16b8e4c.

Please sign in to comment.