archlinux / contrib

Arch contrib scripts
GNU General Public License v2.0
63 stars 18 forks source link

Added double quotes to vars and exit for commands. #37

Closed denisse-dev closed 3 years ago

denisse-dev commented 3 years ago
  1. Why is this change neccesary? Because we needed to double quote the variables we're using to prevent globbing and word splitting and we needed to add return values when executing a command in case the command fails.

  2. How does it address the issue? By double quoting the variables and adding an OR to the commands that can fail.

  3. What side effects does this change have? Closes #36.

eli-schwartz commented 3 years ago

Shellcheck is wrong... or rather, it is warning you about things that "may or may not be a problem", but aren't a problem here. The variables you're protecting are PyPI names and version numbers, and neither can contain spaces.

If users try to use spaces, then

...

It's simply incorrect no matter what to exit the quoting context for the dots inside of strings that you're adding quotes to. They do not need to avoid a quoting context as far as either shellcheck or the shell grammar are concerned, and continually entering and exiting a quoting context leads to extremely hard to read code.

If you are going to add unneeded but harmless quotes, please just add one set of quotes for each string, not one set of quotes for each variable.

eli-schwartz commented 3 years ago

Thanks. The updated PR produces a much less noisy diff.

Squashed the changes.