[m-rev.] for review: make test hard_coded/string_class test more comprehensive

[Julien Fischer]

On Mon, 21 Sept 2026 at 00:38, Zoltan Somogyi <zoltan.somogyi at runbox.com> wrote:
>
>
>
> On Sun, 20 Sep 2026 20:08:30 +1000, Julien Fischer <jfischer at opturion.com> wrote:
> > Make the hard_coded/string_class test more comprehensive.
>
> For total rewrites like this, sending just the new version is easier
> on reviewers than the diff.
>
> > - Test each test predicate on every test string.
> > - Include is_empty/1 is_all_alnum/1 in the predicates to test.
> > - Add some tests the boundaries of the ASCII range.
> > - Add some tests involving ill-formed strings.
> >
> > Generate the output as a table for ease of comparison.
>
> That's a set of big steps forward.
>
> > +empty  = is_all_empty      alpha_ = is_all_alpha_or_underscore
> > +alpha  = is_all_alpha      alnum_ = is_all_alnum_or_underscore
> > +alnum  = is_all_alnum      match  = all_match(ascii_alpha)
> > +digit  = is_all_digits     nonasc = all_match(nonascii)
>
> These should have shorter abbreviations (such as AN_ or even _N
> for is_all_alnum_or_underscore) to allow four or even three chars
> per column. As is, each row appears split across two lines, at least
> in both my email readers.

Done.

> > --- a/tests/hard_coded/string_class.m
> > +++ b/tests/hard_coded/string_class.m
> > @@ -1,6 +1,20 @@
> >  %---------------------------------------------------------------------------%
> >  % vim: ts=4 sw=4 et ft=mercury
> >  %---------------------------------------------------------------------------%
> > +%
> > +% Test the string predicates that test if every code point in a string
> > +% belongs to some class of characters.
>
> s/if/whether/

Fixed.

> > +% Each row of the output gives the result of every such predicate on one
> > +% test string.
> > +%
> > +% The .exp file is for backends using UTF-8 string encoding.
> > +% The .exp2 file is for backends using UTF-16 string encoding.
>
> s/UTF-X/UTF-X as the/

Done.

> > +% The two outputs differ only in the rows for ill-formed strings: on UTF-8
> > +% backends an ill-formed code unit is a lone byte, while on UTF-16 backends
> > +% it is an unpaired surrogate code point.
>
> Add a comma after both occurrences of  "backends".

Done.

> > +test_string(Str, !IO) :-
> > +    write_row(describe(Str),
> > +        tf(is_empty(Str)),
> > +        tf(is_all_alpha(Str)),
> > +        tf(is_all_alnum(Str)),
> > +        tf(is_all_alpha_or_underscore(Str)),
> > +        tf(is_all_alnum_or_underscore(Str)),
> > +        tf(is_all_digits(Str)),
> > +        tf(all_match(ascii_alpha, Str)),
> > +        tf(all_match(nonascii, Str)),
> > +        !IO).
> > +
> > +:- pred write_row(string::in, string::in, string::in, string::in, string::in,
> > +    string::in, string::in, string::in, string::in, io::di, io::uo) is det.
> > +
> > +write_row(Str, Empty, Alpha, Alnum, AlphaU, AlnumU, Digit, Match, NonAscii,
> > +        !IO) :-
>
> The first argument here is not really Str, it is describe(Str). I would
> therefore rename it as Desc.

Done.

> I would put the table of test strings at the end of the module.

Done.

> Other than that, the diff is fine.

Thanks for that.

Julien.