Re: [PATCH] In string-split, add support for character sets and predicates.

[Daniel Hartwig]

Hi Mark

Thanks for the speedy response.

General ACK on all your comments.  Some additional notes:

On 8 October 2012 23:40, Mark H Weaver <address@hidden> wrote:
>> [which vs. that]

This was straight out of the doc string for string-index.  Changed.

>> +  if (SCM_CHARP (char_pred))
>> +    {
>> +      goto split_char;
>
> I'd prefer to avoid the use of 'goto' here, and instead use nested 'if's
> here.  (I admit that 'goto's occasionally make code simpler and more
> readable, but not in this case IMO).

I had done this only to avoid diff noise on the original code path
since I wasn't changing it.  I agree that it adds nothing to the
structure of the code.  Changed.

> Can you please put the code between 'split_char' and 'done' within this
> 'if', and all the code from here to 'split_char' within the 'else'?

Done.  Also adjusted the scope of the branch-specific variables.

Updated version attached.

Regards

0001-In-string-split-add-support-for-character-sets-and-p.patch
Description: Binary data