run shellcheck (#97)

* run shellcheck

* Update .github/workflows/shellcheck.yml

Co-authored-by: Conrad Ludgate <oon@conradludgate.com>

* shellcheck fixes in which i am highly confident

* shellcheck fixes in which I am less confident - not executing this is fine, right?

* SC2155

In src/shell/atuin.bash line 1:
export ATUIN_SESSION=$(atuin uuid)
       ^-----------^ SC2155: Declare and assign separately to avoid masking return values.

* shellcheck doesn't support zsh, per https://github.com/koalaman/shellcheck/wiki/SC1071

* yaml fix

* gotta checkout our code, too

* yaml fix

* action spelling

* exclude .zsh

* Shellcheck doesn't support zsh, per

https://github.com/koalaman/shellcheck/wiki/SC1071, and the ignore: param
in ludeeus/action-shellcheck only supports _directories_, not _files_.
So instead, we manually add any error the shellcheck step finds in the
file to the above line ...

* comment all the ignores

* Update src/shell/atuin.bash

Co-authored-by: Ellie Huxtable <ellie@elliehuxtable.com>

* new zsh plugin shellcheck errors

* new zsh plugin shellcheck errors, pt 2

Co-authored-by: Conrad Ludgate <oon@conradludgate.com>
Co-authored-by: Ellie Huxtable <ellie@elliehuxtable.com>

.github/workflows/shellcheck.yml

@@ -0,0 +1,18 @@
+name: Shellcheck
+
+on:
+  push:
+    branches: [ main ]
+  pull_request:
+    branches: [ main ]
+
+jobs:
+  shellcheck:
+    runs-on: ubuntu-latest
+
+    steps:
+    - uses: actions/checkout@v2
+    - name: Run shellcheck
+      uses: ludeeus/action-shellcheck@master
+      env:
+        SHELLCHECK_OPTS: "-e SC2148"

atuin.plugin.zsh

@@ -1,3 +1,4 @@
+# shellcheck disable=2148,SC2168,SC1090,SC2125
 local FOUND_ATUIN=$+commands[atuin]
 
 if [[ $FOUND_ATUIN -eq 1 ]]; then

install.sh

@@ -23,7 +23,9 @@ Please file an issue if you encounter any problems!
 EOF
 
 LATEST_RELEASE=$(curl -L -s -H 'Accept: application/json' https://github.com/ellie/atuin/releases/latest)
-LATEST_VERSION=$(echo $LATEST_RELEASE | sed -e 's/.*"tag_name":"\([^"]*\)".*/\1/')
+# Allow sed; sometimes it's more readable than ${variable//search/replace}
+# shellcheck disable=SC2001
+LATEST_VERSION=$(echo "$LATEST_RELEASE" | sed -e 's/.*"tag_name":"\([^"]*\)".*/\1/')
 
 __atuin_install_arch(){
 	echo "Arch Linux detected!"
@@ -53,7 +55,7 @@ __atuin_install_ubuntu(){
 	ARTIFACT_URL="https://github.com/ellie/atuin/releases/download/$LATEST_VERSION/atuin_${LATEST_VERSION//v/}_amd64.deb"
 
 	TEMP_DEB="$(mktemp)" &&
-	wget -O "$TEMP_DEB" $ARTIFACT_URL
+	wget -O "$TEMP_DEB" "$ARTIFACT_URL"
 	sudo dpkg -i "$TEMP_DEB"
 	rm -f "$TEMP_DEB"
 }
@@ -62,7 +64,7 @@ __atuin_install_linux(){
 	echo "Detected Linux!"
 	echo "Checking distro..."
 
-	if $(uname -a | grep -qi "Microsoft"); then
+	if (uname -a | grep -qi "Microsoft"); then
         OS="UbuntuWSL"
     else
         if ! command -v lsb_release &> /dev/null; then
@@ -73,9 +75,9 @@ __atuin_install_linux(){
         OS=$(lsb_release -i | awk '{ print $3 }')
     fi
 
-	if [ $OS == "Arch" ] || [ $OS == "ManjaroLinux" ]; then
+	if [ "$OS" == "Arch" ] || [ "$OS" == "ManjaroLinux" ]; then
 		__atuin_install_arch
-    elif [ $OS == "Ubuntu" ] || [ $OS == "Debian" ] || [ $OS == "Linuxmint" ] || [ $OS == "Parrot" ] || [ $OS == "Kali" ] || [ $OS == "Elementary" ]; then
+    elif [ "$OS" == "Ubuntu" ] || [ "$OS" == "Debian" ] || [ "$OS" == "Linuxmint" ] || [ "$OS" == "Parrot" ] || [ "$OS" == "Kali" ] || [ "$OS" == "Elementary" ]; then
 		__atuin_install_ubuntu
 	else
 		# TODO: download a binary or smth
@@ -126,7 +128,7 @@ __atuin_install_unsupported(){
 	echo "If you have any problems, please open an issue!"
 
 	while true; do
-		read -p "Do you wish to attempt an install with `cargo`?" yn
+		read -r -p "Do you wish to attempt an install with 'cargo'?" yn
 		case $yn in
 			[Yy]* ) __atuin_install_cargo; break;;
 			[Nn]* ) exit;;
@@ -138,16 +140,20 @@ __atuin_install_unsupported(){
 # TODO: would be great to support others!
 case "$OSTYPE" in
   linux*)   __atuin_install_linux ;;
-  darwin*)  __atuin_install_mac ;; 
+  darwin*)  __atuin_install_mac ;;
   msys*)    __atuin_install_unsupported ;;
   solaris*) __atuin_install_unsupported ;;
   bsd*)     __atuin_install_unsupported ;;
   *)        __atuin_install_unsupported ;;
 esac
 
-# TODO: Check which is in use
+# TODO: Check which shell is in use
+# Use of single quotes around $() is intentional here
+# shellcheck disable=SC2016
 printf '\neval "$(atuin init zsh)"' >> ~/.zshrc
 
 curl https://raw.githubusercontent.com/rcaloras/bash-preexec/master/bash-preexec.sh -o ~/.bash-preexec.sh
 printf '\n[[ -f ~/.bash-preexec.sh ]] && source ~/.bash-preexec.sh' >> ~/.bashrc
+# Use of single quotes around $() is intentional here
+# shellcheck disable=SC2016
 echo 'eval "$(atuin init bash)"' >> ~/.bashrc

src/shell/atuin.bash

@@ -1,4 +1,5 @@
-export ATUIN_SESSION=$(atuin uuid)
+ATUIN_SESSION=$(atuin uuid)
+export ATUIN_SESSION
 
 _atuin_preexec() {
 	id=$(atuin history start "$1")

src/shell/atuin.zsh

@@ -1,3 +1,11 @@
+# shellcheck disable=SC2034,SC2153,SC2086,SC2155
+
+# Above line is because shellcheck doesn't support zsh, per
+# https://github.com/koalaman/shellcheck/wiki/SC1071, and the ignore: param in
+# ludeeus/action-shellcheck only supports _directories_, not _files_. So
+# instead, we manually add any error the shellcheck step finds in the file to
+# the above line ...
+
 # Source this in your ~/.zshrc
 autoload -U add-zsh-hook