git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: git@vger.kernel.org
Cc: Jeff King <peff@peff.net>
Subject: Re: [PATCH 1/2] test-lib-functions: make packetize() more efficient
Date: Sat, 28 Mar 2020 20:05:10 -0700	[thread overview]
Message-ID: <xmqq8sjjdfo9.fsf@gitster.c.googlers.com> (raw)
In-Reply-To: <xmqqh7y8c94g.fsf@gitster.c.googlers.com> (Junio C. Hamano's message of "Sat, 28 Mar 2020 17:11:59 -0700")

Junio C Hamano <gitster@pobox.com> writes:

I guess I didn't give an explicit conclusion in my message.

>>  - it's in POSIX, at least as far back as 2004 (I couldn't find an easy
>>    copy of the 2001 version). That doesn't prove there aren't
>>    problematic systems, of course, but it at least passes the bar of
>>    "not even in POSIX".
>
> Yeah, IIRC the list was written in response to a request for _some_
> guidance, so it largely came from in-house rules of my previous
> life, back when I had to deal with various flavours of UNIXen.

I strongly suspect that most of these historical curiosity systems
died out or learned ${#posix}; I wouldn't at all be surprised if
/bin/sh on Solaris back then was one of the motivating systems that
led to the forbidding of the use of ${#parameter} in our in-house
rules, but luckily, we have written it off as unsalvageable in this
project ;-)

It has been in POSIX long enough, and it is useful at times, so
let's drop it from the list of guidelines (patch?).

>>  - it's not in check-non-portable-shell.pl. :) That doesn't mean
>>    CodingGuidelines is wrong, but we should probably reconcile them.
>
> That checker came much much later than the guidelines so it is not
> surprising at all for it to be "buggy", in the sense that it does
> not check everything the guidelines ask.  Yes, we may need bugfixes
> and there may be other bugs, too.

This still gives us something to keep an eye on.

-- >8 --
Subject: CodingGuidelines: allow ${#posix} == strlen($posix)

The construct has been in POSIX for the past 10+ years, and we have
used in t9xxx (subversion) series of the tests, so we know it is at
least portable across systems that subversion Perl bindings have
been ported to.

Let's loosen the rule; luckily, the check-non-portable-shell script
does not have any rule to find its use, so the only change needed is
a removal of one paragraph from the documentation.

Helped-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---

 Documentation/CodingGuidelines | 2 --
 1 file changed, 2 deletions(-)

diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines
index ed4e443a3c..390ceece52 100644
--- a/Documentation/CodingGuidelines
+++ b/Documentation/CodingGuidelines
@@ -91,8 +91,6 @@ For shell scripts specifically (not exhaustive):
 
    - No shell arrays.
 
-   - No strlen ${#parameter}.
-
    - No pattern replacement ${parameter/pattern/string}.
 
  - We use Arithmetic Expansion $(( ... )).

  reply	other threads:[~2020-03-29  3:05 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-03-27  8:02 [PATCH 0/2] upload-pack: handle unexpected v2 delim packets Jeff King
2020-03-27  8:03 ` [PATCH 1/2] test-lib-functions: make packetize() more efficient Jeff King
2020-03-27 15:16   ` Taylor Blau
2020-03-28 12:25     ` Jeff King
2020-03-27 19:18   ` Junio C Hamano
2020-03-28 11:20     ` Jeff King
2020-03-29  0:11       ` Junio C Hamano
2020-03-29  3:05         ` Junio C Hamano [this message]
2020-03-29 14:53           ` Jeff King
2020-03-29 15:44             ` Junio C Hamano
2020-03-29 14:52         ` Jeff King
2020-03-29 15:02       ` [PATCH] test-lib-functions: simplify packetize() stdin code Jeff King
2020-03-29 15:49         ` Junio C Hamano
2020-03-27  8:03 ` [PATCH 2/2] upload-pack: handle unexpected delim packets Jeff King
2020-03-27 15:17   ` Taylor Blau
2020-03-27 15:18 ` [PATCH 0/2] upload-pack: handle unexpected v2 " Taylor Blau

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=xmqq8sjjdfo9.fsf@gitster.c.googlers.com \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=peff@peff.net \
    /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).