Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: shellcheck's SC2086 issue: Double quote to prevent globbing and word splitting #903

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

rasa
Copy link
Contributor

@rasa rasa commented Oct 7, 2023

Fixes SC2086: (info): Double quote to prevent globbing and word splitting.

Here is a summary of the other issues shellcheck reports, per
shellcheck deb-get | grep -o -P 'SC\d\d\d\d' | sort | uniq -c | sort -nr:

    183 SC2292
     66 SC2312
     12 SC2155
     12 SC2076
      8 SC2250
      8 SC2249
      7 SC2207
      4 SC2206
      4 SC2034
      3 SC2178
      3 SC2128
      2 SC2164
      2 SC2046
      2 SC2044
      2 SC1090
      1 SC2154
      1 SC2053
      1 SC2016

Let me know if you are open to my submitting PRs for any or all of these issues. I will submit granular PRs, unless advised otherwise.

Depending on the issue, the PR may be as simple as placing shellcheck disable=SCnnnn at the top of the file.

@philclifford
Copy link
Member

This is looking a lot better since some recent merges and re-factoring happened. See also #871 and #886 for related efforts. Currently we are here :

shellcheck deb-get | grep -o -P 'SC\d\d\d\d' | sort | uniq -c | sort -nr
     23 SC2076
     22 SC2086
      8 SC2155
      5 SC2034
      3 SC2064
      2 SC2164
      2 SC1090
      1 SC2206
      1 SC2016

Some pending PRs will also cause/receive conflicts from this one even if re-based now but such is life ...

Your PRs will be very welcome. (Especially since I can possibly then merge your PRs and deal with any conflicts in mine!). I'd suggest SC2155 and SC2076 are candidates for ignoring generally (or granular PRs with discussion)

Some warnings concern "intended" behaviours.

Globally disabling checks is problematic, so I'd suggest generally command/block-level exclusions are always to be preferred.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants