On 15/10/2015 21:21, Marvin Humphrey wrote:
On Wed, Oct 7, 2015 at 4:10 AM, Nick Wellnhofer <[email protected]> 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.
OK.
- 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.
+1
* Remove `new_from_char`.
-0.5
* Remove `BaseX_To_I64`, optionally adding its functionality to
StringHelper.
+0
* 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`.
+1
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.
Nick
