3 ms·

> and there's a lot of power/potential in some of the weird stuff Shellcheck walls off. Can you give an example? Something Shellcheck warns you away from but w

by jonahx 6y ago

> and there's a lot of power/potential in some of the weird stuff Shellcheck walls off. Can you give an example? Something Shellcheck warns you away from but which, if you used it, would be better (more expressive / more powerful / more whatever) than a shellcheck-approved solution?

abathur 6y ago
Intentional word-splitting is a common one. So, for example: extract_nix_version(){ echo "$3" } get_nix_version(){ # shellcheck disable=SC2046 extract_nix_version $(nix --version) } $ nix --version nix (Nix) 2.3.4 $ get_nix_version 2.3.4 This gets the information without another external utility (grep, cut, sed, awk...) It's well-past bedtime, so for now I'm just leaving the first one I can think of (it's top-of-mind since I Shellchecked it in the last couple weeks... :])
- abathur 6y ago
  It's a little contrived in this case, but another common example is a related suggestion for `read -r`: get_nix_version_parts(){ local major minor patch # shellcheck disable=SC2034,SC2162 IFS="." read major minor patch < <(get_nix_version) local -p } $ get_nix_version_parts major=2 minor=3 patch=4
  - jonahx 6y ago
    Using -r there works fine for me, though your SC2034 complaint is valid, in that it essentially prevents from you from using a very specific built in formatter provided by "local -p". This feels like a rare case, and I'd have no problem doing the "disable=SC2034" there, but I'd add a note explaining why. Even then, one could make an argument (I think I would, actually) that an explicit printf is clearer and more robust. For example, it gives you more control over the formatting, and it doesn't introduce implicit coupling between the output of "local -p" and our function, which in theory could change in the future or be different on different bash versions, etc. I suspect this probability is very low in the particular case of local -p, but still it's not a great practice. Not something I want to give the stamp of approval of inside my code base. get_nix_version_parts(){ local major minor patch IFS="." read -r major minor patch < <(nixv) printf '%s=%s\n' major "$major" minor "$minor" patch "$patch" }
    - abathur 6y ago
      Good catch on -r not mattering, here. I think `read` is one I've had trouble with and assumed that was it since I bothered disabling it. But after grepping around a bit more, the only one I see that shellcheck would object to that actually differs isn't common enough that I'd gripe about it. I don't actually have a gripe with 2034, here. It's a good example of a statement that the user obviously has to triage. Likewise, I'm just using `local -p` as a cheap way to show that the variables populate, it wasn't part of the original code I adapted. Yes--the case for coupling to the output of local -p is when the output is going to be used to set variables again later.
- jonahx 6y ago
  Here is shellcheck compliant solution which uses no external utilities, and is explicit about our intention: # function to imitate "nix --version" since I don't have it installed nixv() { echo 'nix (Nix) 2.3.4' } read -ra arr < <(nixv) echo "${arr[2]}"
- bitsig 6y ago
  > get_nix_version(){ > # shellcheck disable=SC2046 > extract_nix_version $(nix --version) > } You might do something like: get_nix_version(){ local IFS=' ' extract_nix_version $(nix --version) } if you want set "IFS=$'\n'" globally, and still be able split on space "locally". Edit: Still needs the "# shellcheck disable=SC2046" stanza, though
abathur 6y ago
Thanks for asking this, by the way. Going back to intentionally look through a few things for examples is helping me better clarify why I've had this impression slowly building up... 1. Some Shellcheck code titles/short messages communicate uncertainty well, but some others can read as more confident/absolute than the situation merits. 2. Broadly, Shellcheck tends to steer people away from word-splitting. Word-splitting can be a surprising PITA, so I get it. But, it's also a critical concept for understanding shell. If you are reluctantly writing Bash/shell, it's good to have Shellcheck help you avoid it. If you want or need to understand the language, you're going to have to grapple with word-splitting to get there.

by jonahx 6y ago

abathur 6y ago

Intentional word-splitting is a common one. So, for example: extract_nix_version(){ echo "$3" } get_nix_version(){ # shellcheck disable=SC2046 extract_nix_version $(nix --version) } $ nix --version nix (Nix) 2.3.4 $ get_nix_version 2.3.4 This gets the information without another external utility (grep, cut, sed, awk...) It's well-past bedtime, so for now I'm just leaving the first one I can think of (it's top-of-mind since I Shellchecked it in the last couple weeks... :])

abathur 6y ago
It's a little contrived in this case, but another common example is a related suggestion for `read -r`: get_nix_version_parts(){ local major minor patch # shellcheck disable=SC2034,SC2162 IFS="." read major minor patch < <(get_nix_version) local -p } $ get_nix_version_parts major=2 minor=3 patch=4
- jonahx 6y ago
  Using -r there works fine for me, though your SC2034 complaint is valid, in that it essentially prevents from you from using a very specific built in formatter provided by "local -p". This feels like a rare case, and I'd have no problem doing the "disable=SC2034" there, but I'd add a note explaining why. Even then, one could make an argument (I think I would, actually) that an explicit printf is clearer and more robust. For example, it gives you more control over the formatting, and it doesn't introduce implicit coupling between the output of "local -p" and our function, which in theory could change in the future or be different on different bash versions, etc. I suspect this probability is very low in the particular case of local -p, but still it's not a great practice. Not something I want to give the stamp of approval of inside my code base. get_nix_version_parts(){ local major minor patch IFS="." read -r major minor patch < <(nixv) printf '%s=%s\n' major "$major" minor "$minor" patch "$patch" }
  - abathur 6y ago
    Good catch on -r not mattering, here. I think `read` is one I've had trouble with and assumed that was it since I bothered disabling it. But after grepping around a bit more, the only one I see that shellcheck would object to that actually differs isn't common enough that I'd gripe about it. I don't actually have a gripe with 2034, here. It's a good example of a statement that the user obviously has to triage. Likewise, I'm just using `local -p` as a cheap way to show that the variables populate, it wasn't part of the original code I adapted. Yes--the case for coupling to the output of local -p is when the output is going to be used to set variables again later.
jonahx 6y ago
Here is shellcheck compliant solution which uses no external utilities, and is explicit about our intention: # function to imitate "nix --version" since I don't have it installed nixv() { echo 'nix (Nix) 2.3.4' } read -ra arr < <(nixv) echo "${arr[2]}"
bitsig 6y ago
> get_nix_version(){ > # shellcheck disable=SC2046 > extract_nix_version $(nix --version) > } You might do something like: get_nix_version(){ local IFS=' ' extract_nix_version $(nix --version) } if you want set "IFS=$'\n'" globally, and still be able split on space "locally". Edit: Still needs the "# shellcheck disable=SC2046" stanza, though

abathur 6y ago

Thanks for asking this, by the way. Going back to intentionally look through a few things for examples is helping me better clarify why I've had this impression slowly building up... 1. Some Shellcheck code titles/short messages communicate uncertainty well, but some others can read as more confident/absolute than the situation merits. 2. Broadly, Shellcheck tends to steer people away from word-splitting. Word-splitting can be a surprising PITA, so I get it. But, it's also a critical concept for understanding shell. If you are reluctantly writing Bash/shell, it's good to have Shellcheck help you avoid it. If you want or need to understand the language, you're going to have to grapple with word-splitting to get there.