From: John Keeping <john@keeping.me.uk>
To: Johannes Schindelin <johannes.schindelin@gmx.de>
Cc: Junio C Hamano <gitster@pobox.com>,
git@vger.kernel.org, Elia Pinto <gitter.spiros@gmail.com>
Subject: Re: [PATCH 0/3] Fix $((...)) coding style
Date: Thu, 4 Feb 2016 12:14:37 +0000 [thread overview]
Message-ID: <20160204121437.GF29880@serenity.lan> (raw)
In-Reply-To: <cover.1454587284.git.johannes.schindelin@gmx.de>
On Thu, Feb 04, 2016 at 01:01:39PM +0100, Johannes Schindelin wrote:
> I noticed through a nearby patch series that was submitted by Elia that
> some of the $((...)) expressions introduced in scripts that I introduced
> to Git's source code did not match the existing code's convention:
> previously these expressions did not contain any spaces, now *some* do.
>
> This patch series tries to clean that up quickly before even more code
> has to decide which one of the disagreeing coding conventions to use.
>
> Note: For the sake of getting this patch series out, I skipped t/ and
> contrib/. I do not care much about the latter, but t/ should probably be
> fixed, too.
Should this be going this other way (i.e. standardising on having the
spaces)?
The current state (excluding contrib/ and t/) seems to favour spaces:
$ git grep '\$((' -- ':/' ':!t/' ':!contrib/'
Documentation/CodingGuidelines: - We use Arithmetic Expansion $(( ... )).
Documentation/CodingGuidelines: of them, as some shells do not grok $((x)) while accepting $(($x))
generate-cmdlist.sh: n=$(($n+1))
git-filter-branch.sh: elapsed=$(($now - $start_timestamp))
git-filter-branch.sh: remaining=$(( ($commits - $count) * $elapsed / $count ))
git-filter-branch.sh: next_sample_at=$(( ($elapsed + 1) * $count / $elapsed ))
git-filter-branch.sh: next_sample_at=$(($next_sample_at + 1))
git-filter-branch.sh: git_filter_branch__commit_count=$(($git_filter_branch__commit_count+1))
git-rebase--interactive.sh: total=$(($new_count + $(git stripspace --strip-comments <"$todo" | wc -l)))
git-rebase--interactive.sh: count=$(($(sed -n \
git-rebase--interactive.sh: lineno=$(( $lineno + 1 ))
git-rebase--merge.sh: msgnum=$(($msgnum + 1))
git-rebase--merge.sh: eval GITHEAD_$cmt='"${cmt_name##refs/heads/}~$(($end - $msgnum))"'
git-rebase--merge.sh: msgnum=$(($msgnum + 1))
git-rebase--merge.sh: msgnum=$(($msgnum + 1))
git-submodule.sh: n=$(($1 + 0)) 2>/dev/null && test "$n" = "$1"
git-submodule.sh: total_commits=" ($(($total_commits + 0)))"
I make that 3 without spaces (including the git-rebase--interactive.sh
case that wraps) and 12 that do have spaces around operators. Using
spaces around operators also matches our C coding style.
next prev parent reply other threads:[~2016-02-04 12:15 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-02-04 12:01 [PATCH 0/3] Fix $((...)) coding style Johannes Schindelin
2016-02-04 12:02 ` [PATCH 1/3] filter-branch: fix style of $((...) construct Johannes Schindelin
2016-02-04 12:02 ` [PATCH 2/3] rebase--interactive: adjust the coding style of $((...)) Johannes Schindelin
2016-02-04 12:03 ` [PATCH 3/3] rebase --merge: adjust $((...)) coding style Johannes Schindelin
2016-02-04 12:14 ` John Keeping [this message]
2016-02-04 12:38 ` [PATCH 0/3] Fix " Johannes Schindelin
2016-02-04 13:01 ` John Keeping
2016-02-04 13:13 ` Johannes Schindelin
2016-02-04 14:06 ` John Keeping
2016-02-04 15:27 ` Johannes Schindelin
2016-02-04 15:53 ` John Keeping
2016-02-04 19:43 ` Junio C Hamano
2016-02-04 19:43 ` Junio C Hamano
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=20160204121437.GF29880@serenity.lan \
--to=john@keeping.me.uk \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=gitter.spiros@gmail.com \
--cc=johannes.schindelin@gmx.de \
/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).