All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: "Đoàn Trần Công Danh" <congdanhqx@gmail.com>
Cc: git@vger.kernel.org, "SZEDER Gábor" <szeder.dev@gmail.com>,
	"Phillip Wood" <phillip.wood123@gmail.com>
Subject: Re: [PATCH v2 2/4] t: remove \{m,n\} from BRE grep usage
Date: Wed, 21 Sep 2022 11:06:56 -0700	[thread overview]
Message-ID: <xmqqpmfo38lb.fsf@gitster.g> (raw)
In-Reply-To: <ebaf6cec07e3a07c969c456e93aa9d4464f75548.1663765176.git.congdanhqx@gmail.com> (=?utf-8?B?IsSQb8OgbiBUcuG6p24gQ8O0bmc=?= Danh"'s message of "Wed, 21 Sep 2022 20:02:30 +0700")

Đoàn Trần Công Danh  <congdanhqx@gmail.com> writes:

> The CodingGuidelines says we should avoid \{m,n\} in BRE usage.
> And their usages in our code base is limited, and subjectively
> hard to read.
>
> Replace them with ERE.

OK.  I do not personally mind allowing \{0,1\} in BRE (which would
give us a portable way to express '?'), but we are not forbidding
ERE in any way, so I am OK with the direction.

> Except for "0\{40\}" which would be changed to "$ZERO_OID",
> which is a better value for testing with:
> GIT_TEST_DEFAULT_HASH=sha256

Absolutely.  This alone is a change worth doing regardless of the
portability issues.

> Signed-off-by: Đoàn Trần Công Danh <congdanhqx@gmail.com>
> ---
>
>  Phillip Wood said:
>  > \{m,n\} is valid in a posix BRE[1]. If we're already using it without
>  > anyone
>  > complaining I think it would be better to update CodingGuidlines to allow
>  > it.
>
>  Yes, I agree. However, I think our usage of \{m,n\} is limited.
>  Let's skip the lifting for now.

OK.

  reply	other threads:[~2022-09-21 18:07 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-09-21 13:02 [PATCH v2 0/4] allow "grep -E", remove {e,f}grep usage Đoàn Trần Công Danh
2022-09-21 13:02 ` [PATCH v2 1/4] CodingGuidelines: allow grep -E Đoàn Trần Công Danh
2022-09-21 13:02 ` [PATCH v2 2/4] t: remove \{m,n\} from BRE grep usage Đoàn Trần Công Danh
2022-09-21 18:06   ` Junio C Hamano [this message]
2022-09-21 13:02 ` [PATCH v2 3/4] t: convert egrep usage to "grep -E" Đoàn Trần Công Danh
2022-09-21 13:02 ` [PATCH v2 4/4] t: convert fgrep usage to "grep -F" Đoàn Trần Công Danh

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=xmqqpmfo38lb.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=congdanhqx@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=phillip.wood123@gmail.com \
    --cc=szeder.dev@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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.