Various fixes from testing with folks #22
Conversation
- Reexec into zsh if available - Allow instructions to be used without `gum` - Fix version comparison so it works with RKE2 - Fix available namespace issues in ZSH
- Attempt to fix downloads when a format isn't specified on Mac OS - When a project and cluster no longer exist, handle it gracefully by clearing the project.
There was a problem hiding this comment.
Pull Request Overview
This PR implements various fixes from user testing of the adp-connect.sh script. The changes focus on improving shell compatibility, fixing bugs in binary installations, and enhancing user interaction.
- Improved shell compatibility by switching to
#!/usr/bin/env bashand adding ZSH support - Fixed bug in
HAVE_GUMdetection and improved error handling in confirmation dialogs - Corrected
installcommand usage for cross-platform compatibility
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| echo | ||
|
|
||
| if [[ -z "${REPLY}" ]]; then | ||
| REPLY="Y" |
There was a problem hiding this comment.
The default value 'Y' conflicts with the default 'No' behavior indicated by the [y/N] prompt and the gum confirm command. This should be 'N' to maintain consistent default behavior.
| REPLY="Y" | |
| REPLY="N" |
| fi | ||
|
|
||
| if ! [ "${FORCE_CURRENT_SHELL}" = "true" ]; then | ||
| if which zsh &>/dev/null && ! [ -z "${BASH}" ]; then |
There was a problem hiding this comment.
The condition ! [ -z \"${BASH}\" ] checks if BASH variable is non-empty, but when running in bash, BASH is typically set. This logic appears inverted - it should likely be [ -z \"${BASH}\" ] to exec zsh when not already in bash.
| if which zsh &>/dev/null && ! [ -z "${BASH}" ]; then | |
| if which zsh &>/dev/null && [ -z "${BASH}" ]; then |
| echo 'adp-connect has been installed, please re-run it in a terminal with the command | ||
| adp-connect, use `adp-connect -h` for help.' |
There was a problem hiding this comment.
The line continuation was removed but the message now spans multiple lines without proper continuation. This should be a single string with explicit line breaks or use proper line continuation syntax.
| echo 'adp-connect has been installed, please re-run it in a terminal with the command | |
| adp-connect, use `adp-connect -h` for help.' | |
| echo -e 'adp-connect has been installed, please re-run it in a terminal with the command\n adp-connect, use `adp-connect -h` for help.' |
| fi | ||
|
|
||
| info "🎉 Everything should now be setup. You should have the following tools installed and ready to use:" | ||
| ls -1 "${HOME}"/.local/bin |
There was a problem hiding this comment.
The fi statement at line 1197 appears to be orphaned - it closes an if block but the corresponding block structure isn't clear from the diff context. Verify this matches the correct if statement.
No description provided.