git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
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 13:01:11 +0000	[thread overview]
Message-ID: <20160204130111.GG29880@serenity.lan> (raw)
In-Reply-To: <alpine.DEB.2.20.1602041334450.2964@virtualbox>

On Thu, Feb 04, 2016 at 01:38:51PM +0100, Johannes Schindelin wrote:
> On Thu, 4 Feb 2016, John Keeping wrote:
> 
> > Using spaces around operators also matches our C coding style.
> 
> Well, by that reasoning you should go the whole nine yards and write
> 
> 	lineno = $(( $lineno + 1 ))
> 
> Except you can't. Because shell code is inherently not like C code.

That isn't my main argument, although I do think the (presumed)
rationale for using spaces in C to improve usability applies here as
well, even if the confines of the language don't let us go as far as in
C.

I'm not actually sure whether spaces inside the enclosing $(( and )) are
helpful, we're much less consistent about that than about spaces around
binary operators.  Having looked at t/ as well now, we really do seem to
favour using spaces around the binary operators so I'm further convinced
this series is switching the wrong cases.

> What I found particularly interesting about 180bad3 (rebase -i: respect
> core.commentchar, 2013-02-11) was that it *snuck in* that coding style: it
> *changed* the existing one (without rationale in the commit message, too).

The last version of that patch I can find in the archive [1] doesn't add
the spaces, so I think they must have been part of Junio's fixup
discusses in the following messages.  Although I don't think the
historic context is useful in deciding which direction to go in the
future.

[1] http://article.gmane.org/gmane.comp.version-control.git/216103

  reply	other threads:[~2016-02-04 13:01 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 ` [PATCH 0/3] Fix " John Keeping
2016-02-04 12:38   ` Johannes Schindelin
2016-02-04 13:01     ` John Keeping [this message]
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=20160204130111.GG29880@serenity.lan \
    --to=john@keeping.me.uk \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --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).