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))
[-- Attachment #1: Type: text/plain, Size: 1081 bytes --] 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. [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: Patch --] [-- Type: text/x-patch, Size: 1314 bytes --] From 3c68014cded797add3daa4030917ab9de299f57d Mon Sep 17 00:00:00 2001 From: Mauro Aranda <maurooaranda@gmail.com> 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
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))))))
...)
Thanks,
--
Basil
"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.
Mauro Aranda <maurooaranda@gmail.com> writes: > Backward-compatible change attached. With this change, all of the > following work: Looks good to me; go ahead and push. Basil's version was shorter, but wasn't as straightforward (and besides, it had an `eval', which always makes me suspicious. :)) -- (domestic pets only, the antidote for overdose, milk.) bloggy blog: http://lars.ingebrigtsen.no
tags 45831 fixed
close 45831 28.1
quit
Lars Ingebrigtsen <larsi@gnus.org> writes:
> Mauro Aranda <maurooaranda@gmail.com> writes:
>
>> Backward-compatible change attached. With this change, all of the
>> following work:
>
> Looks good to me; go ahead and push.
>
> Basil's version was shorter, but wasn't as straightforward (and besides,
> it had an `eval', which always makes me suspicious. :))
Thanks; I've pushed my patch. If someone wants to tweak it to make it
prettier, I don't object.
> It seems like this message and the other one you sent to this bug > address didn't reach the bugtracker, perhaps because the bug is archived > (I didn't have the time to read the docs about that). Would you mind > resending them, but unarchiving the bug first? Surprisingly I didn't get any message back warning me about the bug being archived, but indeed it seems it was archived. I just unarchived it, but I don't think my messages were important enough to deserve a resend. >>> Feel free to drop the 2nd cond clause if backward-compatibility is not >>> deemed too important here. >> It seems it's only backward compatible with a calling convention that >> was never documented, so it's probably not necessary, indeed. > Either way, I do agree with you that it's probably not necessary, but I > wanted the patch to be simple so the bug reported would get fixed > quickly. That part is done, so feel free to drop that sneaky backquote ;-). Wise strategy, Stefan