bug#45831: 28.0.50; list-colors-display callback arg needs to evaluate to a function?

[Mauro Aranda <maurooaranda@gmail.com>]

"Basil L. Contovounesios" <contovob@tcd.ie> writes:

> Mauro Aranda <maurooaranda@gmail.com> writes:
>
>>  (defun list-colors-print (list &optional callback)
>>    (let ((callback-fn
>> -	 (if callback
>> -	     `(lambda (button)
>> -		(funcall ,callback (button-get button 'color-name))))))
>> +         ;; Expect CALLBACK to be a function, but allow it to be a form that
>> +         ;; evaluates to a function, for backward-compatibility.  (Bug#45831)
>> +         (cond ((functionp callback)
>> +                (lambda (button)
>> +                  (funcall callback (button-get button 'color-name))))
>> +               (callback
>> +                `(lambda (button)
>> +                  (funcall ,callback (button-get button 'color-name)))))))
>
> Why not a single evaluated closure, e.g. like the following?
>
>   (let ((callback-fn
>          (when callback
>            ;; Expect CALLBACK to be a function, but allow it to be a form that
>            ;; evaluates to a function, for backward-compatibility (bug#45831).
>            (or (functionp callback)
>                (setq callback (eval callback lexical-binding)))
>            (lambda (button)
>              (funcall callback (button-get button 'color-name))))))
>     ...)

Just because I didn't want to change the original code too much, in case
someone thought the backward-compatible change wasn't necessary.  But of
course, your sample code looks better.

> Thanks,

Thanks for taking a look.