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

Multiple unquoted blanks in echo #377

Closed
sjvudp opened this issue May 19, 2015 · 12 comments
Closed

Multiple unquoted blanks in echo #377

sjvudp opened this issue May 19, 2015 · 12 comments

Comments

@sjvudp
Copy link

sjvudp commented May 19, 2015

Maybe you want to add this: Multiple unquoted blanks in echo are not flagged, like this:

echo Looking around...       done

(Multiple blanks will be condensed to one)

@arth1
Copy link

arth1 commented Jul 24, 2015

That may be what you want. echo is often used to strip spaces in bourne shell, like

freespace=$(echo $(df -P / | tail -1) | cut -d' ' -f4)

@deryni
Copy link

deryni commented Jul 24, 2015

It may be what you want but it isn't safe in general and if you know you want it then you can tell shellcheck to not warn about it with a directive.

@brother
Copy link
Collaborator

brother commented Jul 24, 2015

@arth1 that line would render shellcheck warnings though. might be usual practice but still not clean enough.

in the case of fetching free space via df using "regular tools" I would turn to awk

df -P / | awk '/^//{print $4}'

for the general case of using echo to clean out spaces you could consider using variable expansions.

@arth1
Copy link

arth1 commented Jul 24, 2015

Keep in mind that awk and bash aren't always available - especially in embedded.
It's more likely that sed is available, but not guaranteed, and this isn't any prettier:

freespace=$(df -P / | tail -1 | sed 's/[[:space:]][[:space:]]*/ /g' | cut -d' ' -f4)

This is better:

freespace=$(df -P / tail -1 | tr -s '[:space:]' | cut -d' ' -f4)

... but will not work if you also need to trim leading or trailing spaces.

@deryni
Copy link

deryni commented Jul 24, 2015

Sure but again in those environments are you virtually guaranteed to be dealing with someone well versed enough to understand the problems and be able to add the shellcheck directive to silence the warning. As opposed to the naive user for whom the whitespace combining is a surprise.

@arth1
Copy link

arth1 commented Jul 24, 2015

I'm not so sure. One bug here was due to exactly this - one user had in the past written a script that used echo to strip spaces, and another one removed a "useless echo" and added quotes because of shellcheck suggesting it, and the script's functionality broke.
Using echo (glob, actually) to strip extra spaces might not be good programming, but it's common enough practice that I think shellcheck should at least consider whether it could be intentional.

@deryni
Copy link

deryni commented Jul 24, 2015

Like we've been saying. It may be common but it is not safe or reliable. Which are the things shellcheck is trying to encourage. It may work for you in your specific examples but it is a bad practice and should be documented (with a directive) as such as far as I'm concerned. The number of people who have problems with this sort of thing on StackOverflow (for example) is enormous.

@koalaman
Copy link
Owner

I fixed the markup of the original post. It's not about word splitting during expansions but about literal whitespace between shell words.

@deryni
Copy link

deryni commented Jul 24, 2015

You know I knew that and yet somehow didn't connect far enough to realize that was a different case entirely. =(
Sorry for the noise.

@koalaman
Copy link
Owner

A warning is now in place whenever there are 4 or more spaces between words passed to echo on the same line. Thanks!

Smaller gaps were not included because having two spaces was common but seemingly unintentional and harmless, and gaps between echo and the first word appeared to be mostly due to people padding their commands to make their arguments line up.

@sjvudp
Copy link
Author

sjvudp commented Jul 27, 2021 via email

@koalaman koalaman changed the title Multiple unquotes blanks in echo Multiple unquoted blanks in echo Jul 27, 2021
@sjvudp
Copy link
Author

sjvudp commented Oct 2, 2021

A warning is now in place whenever there are 4 or more spaces between words passed to echo on the same line. Thanks!

Smaller gaps were not included because having two spaces was common but seemingly unintentional and harmless, and gaps between echo and the first word appeared to be mostly due to people padding their commands to make their arguments line up.

But echo a b will be the same as echo a b, but I doubt that the majority deliberately aligns the arguments on echo when the output will be different.

Note: Gitlab messes up the multiple spaces in the code example above: The first one used two spaces between a and b.

Kangie pushed a commit to Kangie/shellcheck that referenced this issue Feb 27, 2023
Merge of upstream through the stable 0.8.0 release.

Conflicts:
	ShellCheck.cabal
	src/ShellCheck/Analytics.hs
	src/ShellCheck/AnalyzerLib.hs

Manual changes:
	Moved maskedReturn isPortageBuild check into Commands.hs

Changelog:
----------------------------------------------------------------
Christian Nassif-Haynes (2):
      Show info about `set -e` suppression during function calls
      Add extra checks for masked return codes

Fabian Wolff (1):
      Do not suggest `grep -c` as a replacement for `grep -l/-L | wc -l`

Jens Petersen (1):
      move readme to extra-doc-files and add changelog to releases

Kamil Cukrowski (1):
      Add a comma to function characters

Matthias Diener (1):
      Clarify 'which'

Rebecca Cran (1):
      Fix typo in SC2006 message: "backticked" vs "backticks"

Vidar Holen (81):
      Merge pull request koalaman#2181 from matthiasdiener/patch-1
      Make x-comparison warning default
      Stable version v0.7.2
      Post-release CHANGELOG update
      Update Cabal version for Hackage
      Add wait between GitHub and Docker to allow replication
      Fix haddock failures (fixes koalaman#2216)
      Treat ${arr[*]} like $* for SC2048
      Fix bad warning for ${#arr[*]}. Fixes koalaman#2218.
      Sanity check command names (fixes koalaman#2227)
      Merge pull request koalaman#2241 from Kamilcuk/master
      Merge pull request koalaman#2238 from bcran/legacy-backticks-msg
      Merge pull request koalaman#2234 from juhp/patch-1
      SC2181: Add '!' in suggestion as appropriate (fixes koalaman#2189)
      Add :/. to chars recognized for \alias suppression (fixes koalaman#2287)
      Don't warn when line starts with &> (fixes koalaman#2281)
      Re-add warnings about 'declare var = value' (fixes koalaman#2279)
      Don't warn about repeated range in [[ -v arr[xxx] ]] (fixes koalaman#2285)
      Don't print colors when $TERM is 'dumb' or unset (fixes koalaman#2260)
      Have SC2155 trigger on 'typeset' as well (fixes koalaman#2262)
      Warn about quoting in assignments to sh declaration utilities (fixes koalaman#1556)
      Fix broken test from previous commit
      Warn about unquoted blanks in echo (fixes koalaman#377)
      Allow printf/return/assignments after exec (fixes koalaman#2249)
      Don't consider [ -n/-z/-v $var ] assignments for subshell modification (fixes koalaman#2217)
      Optionally suggest [[ over [ in Bash scripts (-o require-double-brackets) (fixes koalaman#887)
      Avoid trigger SC2181 on composite $? checks (fixes koalaman#1167)
      Warn about eval'ing arrays
      Merge pull request koalaman#2289 from nafigator/master
      SC2295 Warn about unquoted variables in PE patterns (fixes koalaman#2290)
      Switch build status badge from TravisCI to GitHub
      Remove defunct SonarQube plugin link (fixes koalaman#2292)
      Extend warnings about spaces around = to 'let'
      Suppress SC2167 when name is "_" (fixes koalaman#2298)
      Improve warnings for bad parameter expansion (fixes koalaman#2297)
      Warn about looping over array values and using them as keys
      Don't warn about variables guarded with :+ (fixes koalaman#2296)
      Recognize wait -p as assigning a variable (fixes koalaman#2179)
      Improve warnings for expr (fixes koalaman#2033)
      Add `rg` to list of commands ignored for SC2016 (fixes koalaman#2209)
      Don't warn about unused variables starting with _ (fixes koalaman#1498)
      Merge pull request koalaman#2307 from a1346054/fixes
      Fix parsing of [$var] (fixes koalaman#2309)
      Allow running this repo as a pre-commit hook
      Revert "Allow running this repo as a pre-commit hook"
      Add pre-commit instructions
      Add shellcheck-precommit hook to README.md
      Improve warnings about unnecessary subshells (fixes koalaman#2169)
      Warn about strings for numerical operators in [[ ]] (fixes koalaman#2312)
      Merge pull request koalaman#2303 from DoxasticFox/set-e-functions
      Allow specifying external-sources=true in shellcheckrc (fixes koalaman#1818)
      Merge pull request koalaman#2318 from FabianWolff/grep-lL-wc-l
      Allow `disable=all` to disable all warnings (fixes koalaman#2323)
      Remove SC1004 (fixes koalaman#2326)
      Suppress SC2094 when both are input redirections (fixes koalaman#2325)
      Don't trigger SC2140 on ${x+"a" "b"} (fixes koalaman#2265)
      Strip lines containing "STRIP" from ./striptests
      Add a `setgitversion` script to update the version string with git
      The removed check was SC1004, not SC1003
      Disable UUOC for cat with unquoted variable (fixes koalaman#2333)
      Don't emit SC2140 when trapped string is /, = or : (fixes koalaman#2334)
      Merge pull request koalaman#2320 from DoxasticFox/set-e-proc-sub
      Mention check-extra-masked-returns in changelog
      Add suggestion level in text for TTY output (fixes koalaman#2339)
      Mention require-double-brackets in CHANGELOG
      Warn about `read foo[i]` expanding as glob (fixes koalaman#2345)
      For `while getopts; do case ..` checks, make sure variable matches
      Skip SC2214 if variable is modified in loop (fixes koalaman#2351)
      Mention known incompatibilities in man page
      Treat typeset similar to declare (fixes koalaman#2354)
      Give more examples of what ShellCheck looks for
      Have quickscripts search for relevant paths (fixes koalaman#2286)
      Warn about [^..] in Dash (fixes koalaman#2361)
      Consider all forms of TA_Assignment to remove spaces (fixes koalaman#2364)
      Include `local -r` in check-extra-masked-returns (fixes koalaman#2362)
      Update release checklist
      Update distro tests
      Update stack resolver
      Update copyright years
      Fix bad version on stable releases
      Stable version 0.8.0

Yancharuk Alexander (2):
      Minor changes in README
      Review fixes in README

a1346054 (2):
      Fix redirect in LICENSE file
      Remove trailing whitespace

 .github/workflows/build.yml            |  27 +-
 .github_deploy                         |   1 -
 CHANGELOG.md                           |  51 +-
 LICENSE                                |   2 +-
 README.md                              |  38 +-
 ShellCheck.cabal                       |  12 +-
 quickrun                               |  10 +-
 quicktest                              |  11 +-
 setgitversion                          |  11 +
 shellcheck.1.md                        |  50 +-
 shellcheck.hs                          |  24 +-
 snap/snapcraft.yaml                    |   4 +-
 src/ShellCheck/AST.hs                  |   1 +
 src/ShellCheck/ASTLib.hs               |  74 ++-
 src/ShellCheck/Analytics.hs            | 841 ++++++++++++++++++++++++++++-----
 src/ShellCheck/AnalyzerLib.hs          | 182 +++++--
 src/ShellCheck/Checker.hs              |  84 +++-
 src/ShellCheck/Checks/Commands.hs      | 271 ++++++++++-
 src/ShellCheck/Checks/ShellSupport.hs  |   6 +-
 src/ShellCheck/Data.hs                 |   4 +-
 src/ShellCheck/Formatter/CheckStyle.hs |   2 +-
 src/ShellCheck/Formatter/Diff.hs       |   5 +-
 src/ShellCheck/Formatter/Format.hs     |  21 +-
 src/ShellCheck/Formatter/GCC.hs        |   2 +-
 src/ShellCheck/Formatter/JSON1.hs      |   2 +-
 src/ShellCheck/Formatter/TTY.hs        |   4 +-
 src/ShellCheck/Interface.hs            |  14 +-
 src/ShellCheck/Parser.hs               |  76 ++-
 stack.yaml                             |   2 +-
 striptests                             |   2 +-
 test/buildtest                         |   2 +
 test/check_release                     |   8 +-
 test/distrotest                        |  10 +-
 test/stacktest                         |   5 +-
 34 files changed, 1553 insertions(+), 306 deletions(-)
 create mode 100755 setgitversion

BUG=b:259131253
TEST=stack test; Compare cros lint checks on ebuilds

Change-Id: I5ca3fb27faa59b2f11369c2a150a419dee289977
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

No branches or pull requests

5 participants