Am Mittwoch, 20. September 2017, 12:50:26 CEST schrieb Heiko Bauke: > Hi,
Hi. > at the moment I am reviewing the shell scrips in the darktable source, > mainly by employing the shellcheck tool. In this way, I found some > minor (manly stylistic) issues. The manly stylistic issues. It's all about beard length, isn't it? ;-) > So far I found only one more serious issue in the statement > > if [ -n "`echo -e $NEW_VERSION | grep Format`" ]; then > > in the file tools/create_version_c.sh. The problem is that POSIX > compliant implementations of the build in echo shell function do not > support options. Thus > > echo -e $NEW_VERSION > > will print out literally »-e« plus the content of the variable > $NEW_VERSION (with the usual parameter expansion rules applied) in some > shells (the dash, for example, which is the default on Debian > derivatives). This, however, might not be intended here. Some > implementations of echo (the bash build in, for example) allow to > specify the option -e to enable the interpretation of backslash escapes. Ack, that looks wrong. The interesting thing is that in the end it doesn't really matter if there is a stray "-e" but it's bad nonetheless. > Thus, I wonder do we really need to enable the interpretation of > backslash escapes here? If yes echo should be replaced by /bin/echo, > otherwise »-e« is probably wrong here. Probably the line > > if [ -n "`echo -e $NEW_VERSION | grep Format`" ]; then > > should be replaced by > > if echo "$NEW_VERSION" | grep -q Format; then > > but I am not sure. I pinged Jeremy who introduced those changes back then. Maybe they were a typo, maybe there was a reason. Let's wait for his input. > Heiko Tobias
signature.asc
Description: This is a digitally signed message part.