git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Cc: Miles Bader <miles@gnu.org>,
	Eygene Ryabinkin <rea-git@codelabs.ru>,
	Pierre Habouzit <madcoder@debian.org>,
	git@vger.kernel.org
Subject: Re: [PATCH] Supplant the "while case ... break ;; esac" idiom
Date: Mon, 24 Sep 2007 09:24:53 -0700	[thread overview]
Message-ID: <7vbqbsav56.fsf@gitster.siamese.dyndns.org> (raw)
In-Reply-To: <Pine.LNX.4.64.0709241502330.28395@racer.site> (Johannes Schindelin's message of "Mon, 24 Sep 2007 15:04:32 +0100 (BST)")

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> On Mon, 24 Sep 2007, Miles Bader wrote:
>> ...
>> In practice that's not an issue though -- every reasonable shell has 
>> test as a builtin these days, so the "works when test is not a builtin" 
>> criteria is really important only for robustness.
>
> AAAAAAAAAAAAAARRRRRGGGHHHHHHHHHHHH!
>
> _Exactly_ the same reasoning can be said about the old code: _every_ 
> reasonable shell can grok the code that used to be there!
>
> <rhetoric-question>
> 	So what exactly was your point again?
> </rhetoric-question>

The points are:

 (1) The code used to be there is known to cause trouble with a
     deployed shell on FreeBSD of some vintage.  It may be true
     that the shell is broken, but it does not matter much to
     the end user on such systems if breakage is in shell or in
     scripts --- the end result is that the user cannot benefit
     from git, and we already happen to know the workaround,
     which does not make the scripts less readable nor less
     portable;

 (2) It's not like people who work on git scripts share the
     exact same style and tradition.  While I do not personally
     think there is much readability improvements between the
     old code and the new code, if more people find the latter
     easier to work with, it's better to switch to the new code
     especially because there is no downside.

     (2-a) Nobody finds the latter less readable nor impossible
           to work with.  Even I am not saying that; I only said
           I do not think it improves.

     (2-b) git is not an educational project. It can be done
           elsewhere in a UNIX history class, not here, to teach
           people that "case ... esac" used to be much more
           preferred over "test" because often the latter was
           not built-in and slower.

Regarding "$# != 0" vs "$# -ne 0", I agree with the patch by
David.  If the variable were "$something_else", then it might
have been better to use the explicitly numeric form, but I think
any seasoned shell people is much more used to see $# and $? (or
$status after an earlier "status=$?") compared with numeric
string with "=" or "!=".

  parent reply	other threads:[~2007-09-24 16:26 UTC|newest]

Thread overview: 45+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-09-21 21:43 [PATCH] Allow shell scripts to run with non-Bash /bin/sh Eygene Ryabinkin
2007-09-21 23:52 ` Junio C Hamano
2007-09-22  0:05   ` David Kastrup
2007-09-22  0:18     ` Junio C Hamano
2007-09-22  0:21       ` Junio C Hamano
2007-09-22  0:26       ` David Kastrup
2007-09-22  3:54   ` Eygene Ryabinkin
2007-09-22  4:06     ` David Kastrup
2007-09-22  6:58       ` Junio C Hamano
2007-09-22  7:34         ` Vineet Kumar
2007-09-22 11:07         ` David Kastrup
2007-09-22  6:54     ` Junio C Hamano
2007-09-22 21:37       ` Adam Flott
2007-09-22  8:32     ` Junio C Hamano
2007-09-23  8:31       ` Eygene Ryabinkin
2007-09-23  8:59       ` David Kastrup
2007-09-23 19:20         ` Junio C Hamano
2007-09-23 19:33           ` David Kastrup
2007-09-23 20:42             ` [PATCH] Supplant the "while case ... break ;; esac" idiom David Kastrup
2007-09-23 22:20               ` Junio C Hamano
2007-09-24  6:22                 ` David Kastrup
2007-09-24 14:24                   ` David Kastrup
2007-09-24 23:31                     ` Junio C Hamano
2007-09-25  6:13                       ` David Kastrup
2007-09-25  6:29                         ` Junio C Hamano
2007-09-25 10:33                           ` Johannes Schindelin
2007-09-25 10:46                           ` Avi Kivity
2007-09-24  6:05               ` Mike Hommey
2007-09-24  6:26                 ` David Kastrup
2007-09-24  6:30                   ` David Symonds
2007-09-24  7:57                     ` David Kastrup
2007-09-24  8:01                       ` Pierre Habouzit
2007-09-24  8:04                         ` Pierre Habouzit
2007-09-24 10:33                           ` Johannes Schindelin
2007-09-24 11:21                             ` Miles Bader
2007-09-24 11:35                               ` Eygene Ryabinkin
2007-09-24 13:45                                 ` Miles Bader
2007-09-24 13:58                                   ` David Kastrup
2007-09-24 14:04                                   ` Johannes Schindelin
2007-09-24 14:10                                     ` David Kastrup
2007-09-24 14:58                                     ` Miles Bader
2007-09-24 16:24                                     ` Junio C Hamano [this message]
2007-09-24 11:39                             ` David Kastrup
2007-09-24  8:29                         ` David Kastrup
2007-09-22  2:33 ` [PATCH] Allow shell scripts to run with non-Bash /bin/sh 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

  List information: http://vger.kernel.org/majordomo-info.html

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

  git send-email \
    --in-reply-to=7vbqbsav56.fsf@gitster.siamese.dyndns.org \
    --to=gitster@pobox.com \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=git@vger.kernel.org \
    --cc=madcoder@debian.org \
    --cc=miles@gnu.org \
    --cc=rea-git@codelabs.ru \
    /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.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

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 read-only IMAP folder(s) and NNTP newsgroup(s).