lucy-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Nick Wellnhofer <>
Subject Re: [lucy-dev] Clownfish String API
Date Sun, 18 Oct 2015 12:01:44 GMT
On 15/10/2015 21:21, Marvin Humphrey wrote:
> On Wed, Oct 7, 2015 at 4:10 AM, Nick Wellnhofer <> wrote:
>> - Make stack strings public?
> I'd say wait for now.  They're truly an expert API -- it's too easy to blow
> the C stack by using them in a loop, etc.


>> - Make `Find` return a size_t.
>>    Requires special value for "not found".
> Hmm, that's a toughie.  If the sentinel is SIZE_MAX, that might not fit in all
> host numeric types.

Good point. The problem is not so much the byte size of the return type but 
the fact that a host language might not support unsigned integers. Maybe we 
should limit string sizes to SSIZE_MAX and make `Find` return an ssize_t. But 
this requires to emulate the ssize_t type on non-POSIX platforms.

> In addition, I'd suggest:
> *   Remove `Str_less_than`, replacing it in PolyLexicon with
>      `Str_Compare_To(a, b) < 0`.
> *   Remove `Str_compare` from the public API.


> *   Remove `new_from_char`.


> *   Remove `BaseX_To_I64`, optionally adding its functionality to
>      StringHelper.


> *   Rename the argument to `Ends_With` from `postfix` to `suffix`.  Same with
>      `Ends_With_Utf8`.
> *   The return values for `Trim`, `Trim_Top`, and `Trim_Tail` should be
>      `incremented`.


>> StringIterator
>> - Rename StrIter_substring to Str_new_from_iter?
> Hmm.  I see where you're going with that, but new_from_iter doesn't
> communicate that there must be a single backing String for the two
> StringIterator arguments.  Maybe `StrIter_crop`?

I'm also OK with either StrIter_substring ot StrIter_crop. I only thought it 
might be clearer to make this function a constructor of the String class.

>> - Make `Starts_With`, `Ends_With` skip over strings?
>>    I think this better matches current usage patterns.
> Somehow I don't understand what you mean.

I mean that `Starts_With` should be named something like `Advance_If_Match` 
and make the iterator move past the prefix if it matches. Currently, most code 
that calls StrIter_Start_With looks like

     if (StrIter_Starts_With(iter, prefix)) {
         StrIter_Advance(iter, prefix_len);

This could be replaced with a simpler and more efficient

     if (StrIter_Advance_If_Match(iter, prefix)) {

>> - Find better names for `Skip_{Next|Prev}_Whitespace`?
> `Advance_Whitespace` and `Recede_Whitespace`?

As a non-native speaker, I'm not really sure.

> While preparing this reply, I also created the patch below with some doc
> tweaks.

Looks good. I will start off with your patch.


View raw message