Am Mittwoch, 20. September 2017, 13:54:39 CEST schrieb jeremy rosen: > Nothing I can recall... It's probably safe to remove afaict
Thanks for confirming. Suggested changes are committed in 3fe3c27490545a7a3967a14a3a38b2e18e626414 Tobias > On Wed, Sep 20, 2017 at 1:51 PM, Tobias Ellinghaus <m...@houz.org> wrote: > > 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 > > ___________________________________________________________________________ > darktable developer mailing list > to unsubscribe send a mail to darktable-dev+unsubscr...@lists.darktable.org
signature.asc
Description: This is a digitally signed message part.
