"Shell" should set some standards
I looked at a few of the "shell scripting" tutorials, and a few things immediately jumped out at me.
- You should either commit to teach POSIX compliant shell scripting, or to teach bash specifically. You shouldn't mix the two, like using bash to write POSIX code in one tutorial, and then have the next section be about arrays, which are a bashism and not POSIX compliant
- All code should be https://shellcheck compliant - note that shellcheck changes it's linting based on the shebang - so if you want posix compliance, use a shebang of
#!/bin/shvs.#!/bin/bashfor bash-specific linting
Some specific examples: https://www.learnshell.org/en/Variables
#!/bin/bash
# Change this code
BIRTHDATE=
Presents=
BIRTHDAY=
# Testing code - do not change it
if [ "$BIRTHDATE" == "Jan 1, 2000" ] ; then
echo "BIRTHDATE is correct, it is $BIRTHDATE"
else
echo "BIRTHDATE is incorrect - please retry"
fi
if [ $Presents == 10 ] ; then
echo "I have received $Presents presents"
else
echo "Presents is incorrect - please retry"
fi
if [ "$BIRTHDAY" == "Saturday" ] ; then
echo "I was born on a $BIRTHDAY"
else
echo "BIRTHDAY is incorrect - please retry"
fi
- encouraging use of global variables - don't do this unless they are meant to be GLOBALS
- using
[instead of[[in bash - using
[instead of((for integer comparison - random casing in variables - one is capitalized, the other two are all caps? Why?
- randomly quoting some, but not all variables
- not using a main function
- this code doesn't even pass https://shellcheck.net/
https://www.learnshell.org/en/Loops
- Very odd that they did use correct bash code by managing arrays and using
"${array[@]}"to loop over the array (although simple output could have been done easier in this case with something likeprintf "My name is %s\n" "${NAMES[@]}"), but they do something you shouldn't be encouraging anyone to do, like looping over the output of ls - this is what shell globbing is for. https://mywiki.wooledge.org/ParsingLs -
[ $COUNT -gt 0 ]again - this is legacy POSIX stuff - if you are using bash, this can instead be((count > 0))- it's odd that they use that format when 2 lines later that use arithmetic expansion inCOUNT=$(($COUNT - 1)), although they use$count, when you don't use$for variables in arithmetic expansion in bash - it is incorrect. See https://tldp.org/LDP/abs/html/arithexp.html
If you want to teach good shell scripting, getting users use to best practices, even if they don't understand them all yet, should be an important thing.
I'd recommend picking a shell standard and following it, for example https://google.github.io/styleguide/shellguide.html
Hi @Gorian. Since I'm not an expert in bash or shell in general, I'd appreciate it if you can send over some pull requests to fix these issues.
Thanks.
@ronreiter Sure, I'd be happy to see what I can do to help :)