git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Johannes Schindelin <Johannes.Schindelin@gmx.de>
To: Elia Pinto <gitter.spiros@gmail.com>
Cc: git@vger.kernel.org
Subject: Re: [PATCH 14/15] git-am.sh: replace using expr for arithmetic operations with the equivalent shell builtin
Date: Thu, 4 Feb 2016 12:14:27 +0100 (CET)	[thread overview]
Message-ID: <alpine.DEB.2.20.1602041204430.2964@virtualbox> (raw)
In-Reply-To: <1454581259-57095-1-git-send-email-gitter.spiros@gmail.com>

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)
	$ 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

  parent reply	other threads:[~2016-02-04 11:14 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-02-04 10:20 [PATCH 14/15] git-am.sh: replace using expr for arithmetic operations with the equivalent shell builtin Elia Pinto
2016-02-04 10:20 ` [PATCH 15/15] t4032-diff-inter-hunk-context.sh: " Elia Pinto
2016-02-04 11:14 ` Johannes Schindelin [this message]
2016-02-04 11:53   ` [PATCH 14/15] git-am.sh: " Elia Pinto
2016-02-04 12:01     ` Johannes Schindelin
2016-02-04 19:33 ` Junio C Hamano
2016-02-04 19:39   ` Eric Sunshine
2016-02-05 12:50   ` Elia Pinto
2016-02-04 19:46 ` Stefan Beller

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=alpine.DEB.2.20.1602041204430.2964@virtualbox \
    --to=johannes.schindelin@gmx.de \
    --cc=git@vger.kernel.org \
    --cc=gitter.spiros@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).