bug#28489: Acknowledgement (27.0.50; eieio-persistent slot type validation should be a bit smarter)

[Eric Abrahamsen]

On 09/26/17 20:05 PM, Noam Postavsky wrote:
> Eric Abrahamsen <eric@ericabrahamsen.net> writes:
>
>>      ((eq (car-safe type) 'or)
>> -     ;; If type is a list, and is an or, it is possibly something
>> -     ;; like (or null myclass), so check for that.
>> -     (let ((ans nil))
>> -       (dolist (subtype (cdr type))
>> -         (setq ans (eieio-persistent-slot-type-is-class-p
>> -                    subtype)))
>> -       ans))
>> +     ;; If type is a list, and is an or, it is possible that
>> +     ;; multiple classes are acceptable, find them all.
>> +     (seq-filter (lambda (elt) (class-p elt)) (cdr type)))
>
> You seem to have removed some recursion here, is that correct?  If yes,
> probably something worth explaining in the commit message.
>
> Minor nitpicks:
> - The lambda could be replaced with just #'class-p.
> - The indentation has a mix of tabs and spaces.

That was a dumb mistake! My only excuse is that this was just a quick
code sketch for purposes of discussion. That's my story...

It should be:

(seq-filter #'eieio-persistent-slot-type-is-class-p (cdr type))

This whole section of code, `eieio-persistent-validate/fix-slot-value'
and its neighbors, feels very seat-of-the-pants to me. I wish `cl-typep'
could handle more of this work. But in the meantime this patch (or
something like it) would at least address the actual bug.

I don't think the tabs were my fault! What's Emacs policy on this?