Re: [PATCH] sequence manipulation functions

[Nicolas Petton]

Stefan Monnier <address@hidden> writes:

>> +(defun seq-filter (pred seq)
>> +  "Return a list of all the elements for which (PRED element) is non-nil in 
>> SEQ."
>> +  (delq 'seq--exclude (seq-map (lambda (elt)
>> +                                (if (funcall pred elt)
>> +                                    elt
>> +                                  'seq--exclude))
>> +                              seq)))
>
> This won't do the right thing if the list includes the symbol
> `seq--exclude' (which could happen if this function is used by, say,
> completion code).  Better do a (defvar seq--exclude (make-symbol "exclude"))
> and then use the value of `seq--exclude' rather than the constant
> `seq--exclude'.

Indeed.

>
>> +  "Reduce the two-arguments function FUNCTION across SEQ, starting with 
>> INITIAL-VALUE.
>
> This first line is too long.  I guess we need to drop the
> "two-arguments" part.

Ok, I will do that.


>
>> I have renamed the library "seq" instead of "sequences".
>
> Good.
>
>> seq-doseq macro, similar to dolist.
>
> Actually seq-doseq is "unusable" on lists: it's O(N^2) instead
> of O(N).

The macro could use to "dolist" if the sequence is a list.

>
> Otherwise, it looks fine, thank you,
>
>
>         Stefan

-- 
Nicolas Petton
http://nicolas-petton.fr