feat: install to $HOME/.local/bin if writes to /usr/local/bin fail#188
feat: install to $HOME/.local/bin if writes to /usr/local/bin fail#188
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #188 +/- ##
==========================================
- Coverage 62.96% 62.93% -0.04%
==========================================
Files 212 212
Lines 21782 21782
==========================================
- Hits 13715 13708 -7
- Misses 7004 7009 +5
- Partials 1063 1065 +2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
zimeg
left a comment
There was a problem hiding this comment.
📫 Notes on changes for the kind reviewers!
scripts/install-dev.sh
Outdated
| slack_cli_url="https://downloads.slack-edge.com/slack-cli/slack_cli_${SLACK_CLI_DEV_VERSION}_macOS_64-bit.tar.gz" | ||
| else | ||
| case "$(uname -m)" in | ||
| x86_64) | ||
| slack_cli_url="https://downloads.slack-edge.com/slack-cli/slack_cli_${SLACK_CLI_DEV_VERSION}_macOS_amd64.tar.gz" | ||
| ;; | ||
| arm64 | aarch64) | ||
| slack_cli_url="https://downloads.slack-edge.com/slack-cli/slack_cli_${SLACK_CLI_DEV_VERSION}_macOS_arm64.tar.gz" | ||
| ;; | ||
| *) |
There was a problem hiding this comment.
👁️🗨️ note: This is a strange diff that indents lines to 8 spaces but also unindents cases within a switch statement!
| fi | ||
|
|
||
| delay 0.3 "📦 Extracting the Slack CLI command binary to $(home_path $slack_cli_bin_path)" | ||
| delay 0.3 "📦 Extracting the Slack CLI command binary to $(home_path "$slack_cli_bin_path")" |
There was a problem hiding this comment.
🐢 note: Variables are surrounded to avoid multiple word escapings
| delay 0.1 "📠 Removing packaged download files from $(home_path "$slack_cli_install_dir/slack-cli.tar.gz")" | ||
| rm "$slack_cli_install_dir/slack-cli.tar.gz" |
There was a problem hiding this comment.
⛰️ note: Outputs to clean up the rm printed above!
scripts/install-dev.sh
Outdated
| if [ $(command -v $SLACK_CLI_NAME) ]; then | ||
| echo -e "" | ||
| echo -e "✨ Success! The Slack CLI (build: $SLACK_CLI_DEV_VERSION) is now installed!" | ||
| echo -e "" | ||
| sleep 0.4 | ||
| if [ -d "/usr/local/bin" ] && [ -w "/usr/local/bin" ]; then | ||
| local_bin_path="/usr/local/bin" | ||
| else | ||
| echo "📁 Manually add the Slack CLI command directory to your shell profile" | ||
| echo " export PATH=\"$slack_cli_install_bin_dir:\$PATH\"" | ||
| local_bin_path="$HOME/.local/bin" | ||
| mkdir -p "$local_bin_path" | ||
| fi | ||
| delay 0.1 "🔗 Adding a symbolic link from $(home_path "$local_bin_path/$SLACK_CLI_NAME") to $(home_path "$slack_cli_bin_path")" | ||
| ln -sf "$slack_cli_bin_path" "$local_bin_path/$SLACK_CLI_NAME" |
There was a problem hiding this comment.
🗣️ note: The big change of this PR moves output checks of a configured CLI to later but not this install_slack_cli function. This is to change if statements that follow-
| case "$(basename "$SHELL")" in | ||
| bash) | ||
| echo -e " echo 'export PATH=\"\$HOME/.local/bin:\$PATH\"' >> ~/.bashrc" | ||
| echo -e " source ~/.bashrc" | ||
| ;; | ||
| fish) | ||
| echo -e " mkdir -p \$HOME/.config/fish" | ||
| echo -e " echo 'fish_add_path \$HOME/.local/bin' >> \$HOME/.config/fish/config.fish" | ||
| echo -e " source \$HOME/.config/fish/config.fish" | ||
| ;; | ||
| zsh) | ||
| echo -e " echo 'export PATH=\"\$HOME/.local/bin:\$PATH\"' >> ~/.zshrc" | ||
| echo -e " source ~/.zshrc" | ||
| ;; | ||
| *) | ||
| echo " export PATH=\"$local_bin_path:\$PATH\"" | ||
| ;; | ||
| esac |
There was a problem hiding this comment.
🔍 note: Here's a combined output of successful install from above, and next steps per shell!
There was a problem hiding this comment.
Love that you went the extra mile with fish support!
There was a problem hiding this comment.
📣 note: The ordering of output sections matches across scripts.
There was a problem hiding this comment.
📣 note: Changes of this file should match the dev script above but review is of course so appreciated.
zimeg
left a comment
There was a problem hiding this comment.
💌 A few more notes on changes for the wonderful reviewers! The installation page found a few updates:
🔗 https://docs.slack.dev/tools/slack-cli/guides/installing-the-slack-cli-for-mac-and-linux
|
|
||
| <details> | ||
| <summary>Troubleshooting</summary> | ||
| <summary>Troubleshooting: Command not found</summary> |
There was a problem hiding this comment.
📚 note: These steps match the common cases included in the script output but might still be useful to include in documentation! I will admit to skimming similar outputs sometimes. To the great err in troubleshooting...
|
|
||
| <details> | ||
| <summary>Optional: customize installation using flags</summary> | ||
| <summary>Optional: Customize installation using flags</summary> |
There was a problem hiding this comment.
| <summary>Optional: Customize installation using flags</summary> | |
| <summary>Optional: customize installation using flags</summary> |
i agree with u but technically this is lower case
There was a problem hiding this comment.
🍁 ramble: The "troubleshooting" section for Windows might soon be revisited with a similar change. I'm making note to investigate these error messages more soon for such a change!
lukegalbraithrussell
left a comment
There was a problem hiding this comment.
tiny docs suggestions but docs looks good otherwise!
zimeg
left a comment
There was a problem hiding this comment.
📝 Note to self in rebasings and reverts around!
mwbrooks
left a comment
There was a problem hiding this comment.
✅ I'm happy with this as-is and throwing an approval on it! Love it, thanks @zimeg for taking an idea to reality!
📝 I tweaked your CHANGELOG to use MacOS macOS and added the XDG specification reference.
😱 Minor typo in the test steps where $ rm /usr/local/bin should be $ rm /usr/local/bin/lack-dev (I updated it in case a community member tries to follow the PR test steps)
🧠 I left a question on whether we can make the manual setup more obvious. My suggestion is quite weak, but maybe @lukegalbraithrussell or @zimeg have better ideas?
scripts/install-dev.sh
Outdated
| if command -v "$SLACK_CLI_NAME" >/dev/null 2>&1; then | ||
| echo -e "📺 Success! The Slack CLI (build: $SLACK_CLI_DEV_VERSION) is now installed!" | ||
| else | ||
| echo -e "📝 To get started, manually add the Slack CLI to your shell path:" |
There was a problem hiding this comment.
question: Is there anything that we can do to make it more clear that there are required next steps to finalize the installation? For example, 📝 Required Manual Setup: copy and paste the follow to add the Slack CLI to your path:" and bold the commands?
| case "$(basename "$SHELL")" in | ||
| bash) | ||
| echo -e " echo 'export PATH=\"\$HOME/.local/bin:\$PATH\"' >> ~/.bashrc" | ||
| echo -e " source ~/.bashrc" | ||
| ;; | ||
| fish) | ||
| echo -e " mkdir -p \$HOME/.config/fish" | ||
| echo -e " echo 'fish_add_path \$HOME/.local/bin' >> \$HOME/.config/fish/config.fish" | ||
| echo -e " source \$HOME/.config/fish/config.fish" | ||
| ;; | ||
| zsh) | ||
| echo -e " echo 'export PATH=\"\$HOME/.local/bin:\$PATH\"' >> ~/.zshrc" | ||
| echo -e " source ~/.zshrc" | ||
| ;; | ||
| *) | ||
| echo " export PATH=\"$local_bin_path:\$PATH\"" | ||
| ;; | ||
| esac |
There was a problem hiding this comment.
Love that you went the extra mile with fish support!
Co-authored-by: lukegalbraithrussell <31357343+lukegalbraithrussell@users.noreply.github.com>
Co-authored-by: Michael Brooks <mbrooks@slack-corp.com>
|
@lukegalbraithrussell @mwbrooks Thank y'all both for the reviews and suggestions in improved wording, as always 📚 ✨ I'm going to merge this now for next release but I remain open to follow ups of outputs and documentation! For now I think this is change is alright to close #147. |

Changelog
Summary
This PR adds a fallback path -
$HOME/.local/bin- for the installation symlink if writes fail for/usr/local/bin.Fixes #147 as a workaround, although this might not be a standard everywhere. It is part of the XDG spec FWIW 📚
Related reference: https://specifications.freedesktop.org/basedir-spec/latest/#variables
Preview
install.mov
Reviewers
These changes can be tested using the following commands:
Notes
Lots of formatting changes follow, but the
install.shandinstall-dev.shscripts should match in outputs. I hope to follow up with an editor config matching these changes 🤖Requirements