rusq / slackdump

Save or export your private and public Slack messages, threads, files, and users locally without admin privileges.
GNU General Public License v3.0
1.51k stars 70 forks source link

Add shellcheck CI check + address its concerns in few bash scripts found #272

Closed yarikoptic closed 7 months ago

yarikoptic commented 7 months ago

I (newbie to slackdump) was looking for incremental backup setup. Ran into bash script -- decided to contribute but also to robustify before I "mass try it" since shellcheck is right in warning (generally ;-) ) about such constructs as ( $(im_channels) ) since they might not handle correctly cases where names have spaces. I do not have proof yet though here that it would apply. Neither I have yet tested in this script that the helper append_to_array works.

but here is a script which demonstrates problem and checks different solutions for it ```shell #!/bin/bash # Disabling shellcheck check here since adding escaped symbols for demo # shellcheck disable=SC2028 function func { echo word echo "phrase with space" echo 'wordwith\n' } function func2 { echo 'last-word\r' } function append_to_array { local array_name="$1" shift #local cmd="$@" # Read output of the command into a temporary array, splitting on newline # shellcheck disable=SC2034 if [ -n "$*" ]; then IFS=$'\n' read -r -d '' -a temp_array < <("$@") else # no command provided -- read from stdin IFS=$'\n' read -r -d '' -a temp_array fi # Use eval to safely append temp_array to the named array eval "$array_name+=(\"\${temp_array[@]}\")" } function show { for i in "${LIST[@]}"; do echo "<$i>" done } echo "Can't use mapfile since not portable -- not available on OSX" echo "Simple array: not good -- would not be good for spaces" LIST=( $(func) ) show echo "Tuned read: not good for appends to array" IFS=$'\n' read -r -d '' -a LIST < <(func) IFS=$'\n' read -r -d '' -a LIST < <(func2) show echo "Custom helper: good -- we will append to last listed but tested to work if there no array yet" append_to_array LIST func show echo "Trying more complicated case with a pipe" LIST=() append_to_array LIST < <(func | tr o X) show ```
rusq commented 7 months ago

Hey @yarikoptic, thanks for your contribution!

rusq commented 7 months ago

I have left some comments in the other PR that you have submitted, if you could have a look at it too: #271