Re: [PATCH 1/4] test-lib-functions: introduce test_line_count_cmd

From: Eric Sunshine <sunshine@sunshineco.com>
To: Phillip Wood <phillip.wood123@gmail.com>
Cc: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>,
	"Đoàn Trần Công Danh" <congdanhqx@gmail.com>,
	"Git List" <git@vger.kernel.org>,
	"Junio C Hamano" <gitster@pobox.com>
Subject: Re: [PATCH 1/4] test-lib-functions: introduce test_line_count_cmd
Date: Sun, 13 Jun 2021 19:43:34 -0400	[thread overview]
Message-ID: <CAPig+cRFjBXpm_WwSGBYu75z8TabCDZodYi=wKgTamB1a3aGmg@mail.gmail.com> (raw)
In-Reply-To: <3ab4f29e-987a-c7de-0cca-a64c3bf4be47@gmail.com>

On Sun, Jun 13, 2021 at 2:18 PM Phillip Wood <phillip.wood123@gmail.com> wrote:
> On 13/06/2021 14:28, Ævar Arnfjörð Bjarmason wrote:
> > On Sat, Jun 12 2021, Đoàn Trần Công Danh wrote:
> >> +       { test "x$1" = "x!" && test $# = 1 ; }
> >> [...]
> >> +    if test "x$1" = "x!"
> >
> > We don't use this test idiom in other places, it's OK to just use "$1" =
> > "!". I think we're past whatever portability concern made that popular
> > in some older shell code in the wild.
>
> Slightly off topic but if anyone is interested in the history of this
> test idiom and why it is no longer needed there is a good article at
> https://www.vidarholen.net/contents/blog/?p=1035

Thanks for the link to the article; it was an interesting read.
However, the article does seem to say that such idioms and care may
still be warranted. In particular, the epilog gives an example which
is still relevant on macOS today. (Indeed, I just tried it and it does
error out as the article states.) Even discounting macOS, it also
talks about such bugs existing as late as 2015, which isn't long ago
by any stretch. (And, as someone whose primary -- indeed only --
development machine is ten years old, some of the other bugs it
mentions -- which existed as recently as ten years ago -- don't seem
all that long ago either.)

At any rate, for those of us who are old-timers, the `"x$foo"` idiom
is habit and only costs a couple extra characters, so I for one have
no problem with its presence in the proposed patch.