Re: [Emacs-diffs] master c13a4df: Remove the calls to `seq-into` from `seq-concatenate`

Re: [Emacs-diffs] master c13a4df: Remove the calls to `seq-into` from `seq-concatenate`

[Stefan Monnier]

>     Remove the calls to `seq-into` from `seq-concatenate`
>     Since most new types of seq would have to be defined as sequences (cons
>     cells or CL structs, mostly), there is no need to convert the seqs to
>     sequences (which can be a fairly expensive operation).

I don't understand this change.  Can't it cause two CL-structs to be
vconcat'd together, which wouldn't make much sense?


        Stefan

Re: [Emacs-diffs] master c13a4df: Remove the calls to `seq-into` from `seq-concatenate`

[Nicolas Petton]

[-- Attachment #1: Type: text/plain, Size: 727 bytes --]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>>     Remove the calls to `seq-into` from `seq-concatenate`
>>     Since most new types of seq would have to be defined as sequences (cons
>>     cells or CL structs, mostly), there is no need to convert the seqs to
>>     sequences (which can be a fairly expensive operation).
>
> I don't understand this change.  Can't it cause two CL-structs to be
> vconcat'd together, which wouldn't make much sense?

The previous version of `seq-concatenate' did not prevent that, since
CL-structs are vectors, thus sequences, and the function was using
`sequencep' to determine if the seq had to be converted or not.

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 512 bytes --]

Re: [Emacs-diffs] master c13a4df: Remove the calls to `seq-into` from `seq-concatenate`

[Stefan Monnier]

>> I don't understand this change.  Can't it cause two CL-structs to be
>> vconcat'd together, which wouldn't make much sense?
> The previous version of `seq-concatenate' did not prevent that, since
> CL-structs are vectors, thus sequences, and the function was using
> `sequencep' to determine if the seq had to be converted or not.

Indeed, but if we intend to work for new "sequence" types defined via
CL-struct, that's still a problem.


        Stefan

Re: [Emacs-diffs] master c13a4df: Remove the calls to `seq-into` from `seq-concatenate`

[Nicolas Petton]

[-- Attachment #1: Type: text/plain, Size: 703 bytes --]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>>> I don't understand this change.  Can't it cause two CL-structs to be
>>> vconcat'd together, which wouldn't make much sense?
>> The previous version of `seq-concatenate' did not prevent that, since
>> CL-structs are vectors, thus sequences, and the function was using
>> `sequencep' to determine if the seq had to be converted or not.
>
> Indeed, but if we intend to work for new "sequence" types defined via
> CL-struct, that's still a problem.

Yes, it is still a problem.  One solution would be to test if the seq is
a cl-struct with, in which case we could use `seq-into'.  But then that
would only fix the cl-struct case, not any other.

Nico

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 512 bytes --]

Re: [Emacs-diffs] master c13a4df: Remove the calls to `seq-into` from `seq-concatenate`

[Stefan Monnier]

> Yes, it is still a problem.  One solution would be to test if the seq is
> a cl-struct with, in which case we could use `seq-into'.  But then that
> would only fix the cl-struct case, not any other.

If we make it into a defmethod, at least it would be possible for
specific cl-structs to fix the problem.


        Stefan

Re: [Emacs-diffs] master c13a4df: Remove the calls to `seq-into` from `seq-concatenate`

[Nicolas Petton]

[-- Attachment #1: Type: text/plain, Size: 551 bytes --]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> Yes, it is still a problem.  One solution would be to test if the seq is
>> a cl-struct with, in which case we could use `seq-into'.  But then that
>> would only fix the cl-struct case, not any other.
>
> If we make it into a defmethod, at least it would be possible for
> specific cl-structs to fix the problem.

It is already, or did you mean something else?  But how would you do it
here?  There can be any number of sequences.

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 512 bytes --]

Re: [Emacs-diffs] master c13a4df: Remove the calls to `seq-into` from `seq-concatenate`

[Stefan Monnier]

>>> Yes, it is still a problem.  One solution would be to test if the seq is
>>> a cl-struct with, in which case we could use `seq-into'.  But then that
>>> would only fix the cl-struct case, not any other.
>> If we make it into a defmethod, at least it would be possible for
>> specific cl-structs to fix the problem.
> It is already, or did you mean something else?  But how would you do it
> here?  There can be any number of sequences.

That's exactly the issue: it needs to be structured so that callers can
usefully override it.

As it is currently defined, the only overriding possible is to provide
new valid values for the `type' argument.

So its default definition should make use of seq-into or something
similar so that new types can provide their own implementation.
E.g., you could define a new

  (cl-defgeneric seq-into-sequence (seq)
    (if (sequencep seq) seq
      (error "Don't know how to turn %S into a sequence" seq)))

and then

  (cl-defgeneric seq-concatenate (type &rest seqs)
    (apply #'cl-concatenate type (mapcar #'seq-into-sequence seqs)))


        Stefan