git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: "René Scharfe" <l.s.r@web.de>
Cc: Git List <git@vger.kernel.org>, Masahiro Yamada <masahiroy@kernel.org>
Subject: Re: [PATCH 1/3] test-ctype: test isascii
Date: Sun, 12 Feb 2023 19:39:32 -0800	[thread overview]
Message-ID: <xmqqttzqcjyj.fsf@gitster.g> (raw)
In-Reply-To: 70f4042a-df30-26e5-55bd-b349cc02c416@web.de

René Scharfe <l.s.r@web.de> writes:

> Am 11.02.23 um 20:48 schrieb Junio C Hamano:
>> René Scharfe <l.s.r@web.de> writes:
>>
>>> Test the character classifier added by c2e9364a06 (cleanup: add
>>> isascii(), 2009-03-07).  It returns 1 for NUL as well, which requires
>>> special treatment, as our string-based tester can't find it with
>>> strcmp(3).  Allow NUL to be given as the first character in a class
>>> specification string.  This has the downside of no longer supporting
>>> the empty string, but that's OK since we are not interested in testing
>>> character classes with no members.
>>
>> I wonder how effective a test we can have by checking a table we use
>> in production (i.e. ctype.c::sane_ctype[]) against another table we
>> use only for testing (i.e. string literals in test-ctype.c), but
>> that is not something new in this series.
>
> What aspect is left uncovered?
>
> Or do you mean that the production table should be made trivially
> readable to avoid having to test at all?

Much closer to the latter but not quite.

Both tables are not all that transparent, and it feels that the
protection by the tests largely depends on the fact that it is less
likely for us to make the same mistake in two "not so crystal clear"
tables for the same byte.

> ... but parsing commit messages and blob
> payloads should perhaps better be done with locale-aware versions
> with multi-byte character support.

Yes, that does make sense but it is orthogonal to what sane_ctype
wants to address, I would think.

  reply	other threads:[~2023-02-13  3:40 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-02-11 13:04 [PATCH 0/3] test-ctype: test all classifiers René Scharfe
2023-02-11 13:12 ` [PATCH 1/3] test-ctype: test isascii René Scharfe
2023-02-11 19:48   ` Junio C Hamano
2023-02-12  9:48     ` René Scharfe
2023-02-13  3:39       ` Junio C Hamano [this message]
2023-02-13 18:37         ` René Scharfe
2023-02-13 19:02           ` Junio C Hamano
2023-02-11 13:12 ` [PATCH 2/3] test-ctype: test islower and isupper René Scharfe
2023-02-11 13:12 ` [PATCH 3/3] test-ctype: test iscntrl, ispunct, isxdigit and isprint René Scharfe
2023-02-13 21:08 ` [PATCH v2 0/3] test-ctype: test all classifiers René Scharfe
2023-02-13 21:09   ` [PATCH v2 1/3] test-ctype: test isascii René Scharfe
2023-02-13 21:10   ` [PATCH v2 2/3] test-ctype: test islower and isupper René Scharfe
2023-02-13 21:12   ` [PATCH v2 3/3] test-ctype: test iscntrl, ispunct, isxdigit and isprint René Scharfe
2023-02-13 21:59   ` [PATCH v2 0/3] test-ctype: test all classifiers 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=xmqqttzqcjyj.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=l.s.r@web.de \
    --cc=masahiroy@kernel.org \
    /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).