Re: [PATCH 1/4] std.pk: Implement strrchr

[Arsen Arsenović]

Hi Jose,

"Jose E. Marchesi" <jemarch@gnu.org> writes:

> Hi Arsen.
> Thanks for the patch.
>
>> This patchset implements the pk_vercmp function, intended for comparing
>> GNU poke version strings, as well as the requisite standard library
>> facilities used in such a string manipulation heavy function.  The
>> implementation in pk_vercmp is dual purpose, in that it both verifies
>> and parses a version string.
>>
>> An alternative implementation could be written that populates a struct
>> with broken out version information, rather than ad-hoc parsing two
>> strings at once as is demonstrated here, as a pk_verparse, with
>> pk_vercmp simply being a helper that returns the lexicographic
>> comparison of two parsed versions.  I had initially opted to write a
>> non-verifying lax version of this function, but bolted strictness onto
>> it later, which is why this is the implementation I am presenting.  If
>> you wish, I can upgrade to the approach described in this paragraph.
>> Other patches are unaffected by this decision, and so, can be reviewed
>> in either case.
>
> I like the pk_parse_version approach you describe there.
>
> The Pk_Version struct in pkl-rt could be something like:
>
> type Pk_Version =
>   struct
>   {
>     /* major.minor[.subminor][-BRANCH-OFFSET-HASH] */
>     uint<8> major;
>     uint<8> minor;
>     uint<8> subminor;
>     string branch : branch in ["dev", "maint"];

This constraint would not actually be accurate.  The branch can be any
value, but ``dev'' and ``maint'' have the special meaning of ``master''
and ``maint/poke-*''.  The branches could be anything.

>     uint<8> offset;
>     string tag;
>   };
>
> And then we could have pk_ver_cmp = (Pk_Version a, Pk_Version b) int<32>.
>
>>
>> Are [1,3] OK for master?  What do you think about 4 (the discussion in
>> the paragraph above?
>
> Just this:
>
>>  @menu
>>  * ltrim::           Remove leading characters.
>>  * rtrim::           Remove trailing characters.
>> -* strchr::          Locate a character in a string.
>> +* strchr::              Locate a character in a string, from the beginning.
>
> Is the indentation there right?

Hmm, might be tabs vs. spaces.  I'll double-check.

>> +* strrchr::         Locate a character in a string, from the end.
>>  @end menu
>
> Other than that, the strrchr patch is OK for master.
> Thanks!


-- 
Arsen Arsenović

signature.asc
Description: PGP signature