Re: [PATCH 14/15] git-am.sh: replace using expr for arithmetic operations with the equivalent shell builtin

From: Elia Pinto <gitter.spiros@gmail.com>
Date: Feb 4, 2016, 11:53 UTC
Message-ID: <CA+EOSB=ky8E7J0Fa26DjgT=5-X9N09=4kRbGvWZo5K7PhwS7hg@mail.gmail.com>
In-Reply-To: <alpine.DEB.2.20.1602041204430.2964@virtualbox>

2016-02-04 12:14 GMT+01:00 Johannes Schindelin <Johannes.Schindelin@gmx.de>:

Show 26 quoted lines

> Hi Elia,
>
> On Thu, 4 Feb 2016, Elia Pinto wrote:
>
>> -                     this=$(expr "$this" + 1)
>> +                     this=$(( "$this" + 1 ))
>
> Why the funny spaces? We do not do that anywhere in the existing code
> except in three places (2x filter-branch, 1x rebase--interactive, all
> three *not* my fault) and in some tests.
>
> Also, I am *pretty* certain that the quotes break this code:
>
>         me@work MINGW64 /usr/src/git (master)
>         $ this=1
>
>         me@work MINGW64 /usr/src/git (master)
>         $ this=$(( "$this" + 1 ))
>         bash: "1" + 1 : syntax error: operand expected (error token is ""1" + 1 ")
>
> Whereas if you do *not* add the superfluous spaces and quotes, it works:
>
>         me@work MINGW64 /usr/src/git (master)
>         $ this=1
>
>         me@work MINGW64 /usr/src/git (master)

Thanks for noticing.

You are right. I ran the test but did not notice mistakes, my fault.

I will resend. Thanks again.

Best

Show 34 quoted lines

>         $ this=$(($this+1))
>
>         me@work MINGW64 /usr/src/git (master)
>         $ echo $this
>         2
>
> Maybe this is only a problem with Bash 4.3.42 in MSYS2, but I do not think
> so.
>
> *Clicketyclick*
>
> Nope. It also happens in Ubuntu's Bash 4.3.42:
>
>         me@ubuntu-vm  ~/git (master)
>         $ this=1
>
>         me@ubuntu-vm  ~/git (master)
>         $ this=$(( "$this" + 1 ))
>         bash: "1" + 1 : syntax error: operand expected (error token is ""1" + 1 ")
>
>         me@ubuntu-vm  ~/git (master)
>         $ bash --version
>         GNU bash, version 4.3.42(1)-release (x86_64-pc-linux-gnu)
>         Copyright (C) 2013 Free Software Foundation, Inc.
>         License GPLv3+: GNU GPL version 3 or later
>         <http://gnu.org/licenses/gpl.html>
>
>         This is free software; you are free to change and redistribute it.
>         There is NO WARRANTY, to the extent permitted by law.
>
> ... which makes me wonder in which environment you tested this?
>
> Ciao,
> Dscho