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

classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|

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

Mauro Aranda
Starting from emacs -Q:

1. Eval the following:
(list-colors-display nil nil
                     (let ((cbuf (current-buffer)))
                       (lambda (color)
                         (when (buffer-live-p cbuf)
                           (message "Picked color %s for buffer %s"
                                    color (buffer-name cbuf))))))

2. Hit RET on any color.  I hit RET in the first one, which is black.

3. Get the following error:
Symbol’s function definition is void: closure


Docstring of list-colors-display says:
If the optional argument CALLBACK is non-nil, it should be a
function to call each time the user types RET or clicks on a
color.  The function should accept a single argument, the color name.

But I'm passing a function and it is erroring out.


If instead the CALLBACK argument is a form that evaluates to a function,
like in:
(list-colors-display nil nil
                     (let ((cbuf (current-buffer)))
                       `(function ,(lambda (color)
                                    (when (buffer-live-p cbuf)
                                      (message "Picked color %s for buffer %s"
                                               color (buffer-name cbuf)))))))

it succeeds with: Picked color black for buffer *scratch*


This is not what is described in the docstring, but I think the code
should be fixed to allow the argument to be like the one provided in
step 1.


In GNU Emacs 28.0.50 (build 4, x86_64-pc-linux-gnu, GTK+ Version 3.22.30, cairo version 1.15.10)
 of 2021-01-12 built on tbb-desktop
Repository revision: c734ba68623279d814e857ddc536421a08c38f34
Repository branch: master
Windowing system distributor 'The X.Org Foundation', version 11.0.12008000
System Description: Ubuntu 18.04.5 LTS

Configured features:
CAIRO DBUS FREETYPE GIF GLIB GMP GNUTLS GSETTINGS HARFBUZZ JPEG
LIBSELINUX LIBXML2 MODULES NOTIFY INOTIFY PDUMPER PNG RSVG SOUND THREADS
TIFF TOOLKIT_SCROLL_BARS X11 XDBE XIM XPM GTK3 ZLIB

Important settings:
  value of $LC_MONETARY: es_AR.UTF-8
  value of $LC_NUMERIC: es_AR.UTF-8
  value of $LC_TIME: es_AR.UTF-8
  value of $LANG: en_US.UTF-8
  locale-coding-system: utf-8-unix

Major mode: Lisp Interaction

Minor modes in effect:
  tooltip-mode: t
  global-eldoc-mode: t
  eldoc-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  tool-bar-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  line-number-mode: t
  transient-mark-mode: t

Load-path shadows:
None found.

Features:
(shadow sort mail-extr emacsbug message rmc puny dired dired-loaddefs
rfc822 mml mml-sec epa derived epg epg-config gnus-util rmail
rmail-loaddefs auth-source cl-seq eieio eieio-core cl-macs
eieio-loaddefs password-cache json map mm-decode mm-bodies mm-encode
mail-parse rfc2231 mailabbrev gmm-utils mailheader sendmail rfc2047
rfc2045 ietf-drums mm-util mail-prsvr mail-utils cl-print debug
backtrace find-func time-date subr-x cl-extra shortdoc
text-property-search seq byte-opt gv bytecomp byte-compile cconv
help-fns radix-tree color help-mode easymenu cl-loaddefs cl-lib
iso-transl tooltip eldoc electric uniquify ediff-hook vc-hooks
lisp-float-type mwheel term/x-win x-win term/common-win x-dnd tool-bar
dnd fontset image regexp-opt fringe tabulated-list replace newcomment
text-mode elisp-mode lisp-mode prog-mode register page tab-bar menu-bar
rfn-eshadow isearch timer select scroll-bar mouse jit-lock font-lock
syntax facemenu font-core term/tty-colors frame minibuffer cl-generic
cham georgian utf-8-lang misc-lang vietnamese tibetan thai tai-viet lao
korean japanese eucjp-ms cp51932 hebrew greek romanian slovak czech
european ethiopic indian cyrillic chinese composite charscript charprop
case-table epa-hook jka-cmpr-hook help simple abbrev obarray
cl-preloaded nadvice button loaddefs faces cus-face macroexp files
window text-properties overlay sha1 md5 base64 format env code-pages
mule custom widget hashtable-print-readable backquote threads dbusbind
inotify dynamic-setting system-font-setting font-render-setting cairo
move-toolbar gtk x-toolkit x multi-tty make-network-process emacs)

Memory information:
((conses 16 113889 8725)
 (symbols 48 7333 1)
 (strings 32 21356 2142)
 (string-bytes 1 682695)
 (vectors 16 12605)
 (vector-slots 8 175429 11483)
 (floats 8 158 307)
 (intervals 56 4536 253)
 (buffers 984 16))



Reply | Threaded
Open this post in threaded view
|

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

Mauro Aranda
tags 45831 patch
quit


Backward-compatible change attached.  With this change, all of the
following work:

(list-colors-display nil nil
                     (let ((cbuf (current-buffer)))
                       (lambda (color)
                         (when (buffer-live-p cbuf)
                           (message "Picked color %s for buffer %s"
                                    color (buffer-name cbuf))))))

(list-colors-display nil nil
                     (let ((cbuf (current-buffer)))
                       `(function ,(lambda (color)
                                    (when (buffer-live-p cbuf)
                                      (message "Picked color %s for buffer %s"
                                               color (buffer-name cbuf)))))))

(list-colors-display nil nil
                     (let ((cbuf (current-buffer)))
                       `(lambda (color)
                          (when (buffer-live-p ,cbuf)
                            (message "Picked color %s for buffer %s"
                                     color (buffer-name ,cbuf))))))

(list-colors-display nil nil
                     (let ((cbuf (current-buffer)))
                       #'(lambda (color)
                          (when (buffer-live-p cbuf)
                            (message "Picked color %s for buffer %s"
                                     color (buffer-name cbuf))))))


Feel free to drop the 2nd cond clause if backward-compatibility is not
deemed too important here.



From 3c68014cded797add3daa4030917ab9de299f57d Mon Sep 17 00:00:00 2001
From: Mauro Aranda <[hidden email]>
Date: Tue, 12 Jan 2021 20:41:49 -0300
Subject: [PATCH] Fix list-colors-print handling of callback arg

* lisp/facemenu.el (list-colors-print): Keeping
backward-compatibility, don't fail when passed a closure object as
CALLBACK.  (Bug#45831)
---
 lisp/facemenu.el | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/lisp/facemenu.el b/lisp/facemenu.el
index 2609397b0d..dc5f8f46ab 100644
--- a/lisp/facemenu.el
+++ b/lisp/facemenu.el
@@ -606,9 +606,14 @@ list-colors-display
 
 (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)))))))
     (dolist (color list)
       (if (consp color)
   (if (cdr color)
--
2.29.2

Reply | Threaded
Open this post in threaded view
|

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

Mauro Aranda
"Basil L. Contovounesios" <[hidden email]> writes:

> Mauro Aranda <[hidden email]> 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.