* bug#60730: 29.0.60; Free variable with :buffer keyword in ert-with-temp-file
@ 2023-01-11 13:50 J.P.
2023-01-13 1:56 ` Stefan Kangas
0 siblings, 1 reply; 15+ messages in thread
From: J.P. @ 2023-01-11 13:50 UTC (permalink / raw)
To: 60730; +Cc: Stefan Kangas
Pretty sure this is just a (functional) typo.
If you put something like
;; -*- lexical-binding: t; -*-
(require 'ert-x)
(ert-deftest test ()
(let (foo)
(ert-with-temp-file f
:suffix ".ext"
:text "hw\n"
:buffer b
(should (equal (setq foo b) (get-file-buffer f))))
(should-not (buffer-live-p foo))))
in a file and byte-compile it, you get
^L
Compiling file /tmp/somefile.el at Wed Jan 11 00:28:11 2023
Entering directory ‘/tmp/’
somefile.el: Warning: reference to free variable ‘buf’
Doing
diff --git a/lisp/emacs-lisp/ert-x.el b/lisp/emacs-lisp/ert-x.el
index 83705ca5b8..7cf60bfeae 100644
--- a/lisp/emacs-lisp/ert-x.el
+++ b/lisp/emacs-lisp/ert-x.el
@@ -496,7 +496,7 @@ ert-with-temp-file
(progn ,@body)
(ignore-errors
,@(when buffer
- (list `(with-current-buffer buf
+ (list `(with-current-buffer ,buffer
(set-buffer-modified-p nil))
`(kill-buffer ,buffer))))
(ignore-errors
seems to make it go away.
In GNU Emacs 29.0.60 (build 1, x86_64-pc-linux-gnu, GTK+ Version
3.24.35, cairo version 1.17.6) of 2023-01-10 built on localhost
Repository revision: 1cbc22b9c7f836f5b3311213dca8afa853513442
Repository branch: emacs-29
Windowing system distributor 'The X.Org Foundation', version 11.0.12014000
System Description: Fedora Linux 36 (Workstation Edition)
Configured using:
'configure --enable-check-lisp-object-type --enable-checking=yes,glyphs
'CFLAGS=-O0 -g3'
PKG_CONFIG_PATH=:/usr/lib64/pkgconfig:/usr/share/pkgconfig'
Configured features:
ACL CAIRO DBUS FREETYPE GIF GLIB GMP GNUTLS GPM GSETTINGS HARFBUZZ JPEG
JSON LCMS2 LIBOTF LIBSELINUX LIBSYSTEMD LIBXML2 M17N_FLT MODULES NOTIFY
INOTIFY PDUMPER PNG RSVG SECCOMP SOUND SQLITE3 THREADS TIFF
TOOLKIT_SCROLL_BARS WEBP X11 XDBE XIM XINPUT2 XPM GTK3 ZLIB
Important settings:
value of $LANG: en_US.UTF-8
value of $XMODIFIERS: @im=ibus
locale-coding-system: utf-8-unix
Major mode: Lisp Interaction
Minor modes in effect:
tooltip-mode: t
global-eldoc-mode: t
eldoc-mode: t
show-paren-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
line-number-mode: t
indent-tabs-mode: t
transient-mark-mode: t
auto-composition-mode: t
auto-encryption-mode: t
auto-compression-mode: t
Load-path shadows:
None found.
Features:
(shadow sort mail-extr emacsbug message mailcap yank-media puny dired
dired-loaddefs rfc822 mml mml-sec password-cache epa derived epg rfc6068
epg-config gnus-util text-property-search time-date subr-x mm-decode
mm-bodies mm-encode mail-parse rfc2231 mailabbrev gmm-utils mailheader
cl-loaddefs cl-lib sendmail rfc2047 rfc2045 ietf-drums mm-util
mail-prsvr mail-utils rmc iso-transl tooltip cconv eldoc paren electric
uniquify ediff-hook vc-hooks lisp-float-type elisp-mode 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 lisp-mode
prog-mode register page tab-bar menu-bar rfn-eshadow isearch easymenu
timer select scroll-bar mouse jit-lock font-lock syntax font-core
term/tty-colors frame minibuffer nadvice seq simple cl-generic
indonesian philippine 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 emoji-zwj charscript charprop case-table epa-hook
jka-cmpr-hook help abbrev obarray oclosure cl-preloaded button loaddefs
theme-loaddefs faces cus-face macroexp files window text-properties
overlay sha1 md5 base64 format env code-pages mule custom widget keymap
hashtable-print-readable backquote threads dbusbind inotify lcms2
dynamic-setting system-font-setting font-render-setting cairo
move-toolbar gtk x-toolkit xinput2 x multi-tty make-network-process
emacs)
Memory information:
((conses 16 36352 11795)
(symbols 48 5149 0)
(strings 32 13148 1515)
(string-bytes 1 374127)
(vectors 16 9320)
(vector-slots 8 148190 16310)
(floats 8 21 26)
(intervals 56 220 0)
(buffers 976 10))
^ permalink raw reply [flat|nested] 15+ messages in thread* bug#60730: 29.0.60; Free variable with :buffer keyword in ert-with-temp-file 2023-01-11 13:50 bug#60730: 29.0.60; Free variable with :buffer keyword in ert-with-temp-file J.P. @ 2023-01-13 1:56 ` Stefan Kangas 2023-01-28 14:13 ` J.P. 0 siblings, 1 reply; 15+ messages in thread From: Stefan Kangas @ 2023-01-13 1:56 UTC (permalink / raw) To: J.P.; +Cc: 60730-done "J.P." <jp@neverwas.me> writes: > Pretty sure this is just a (functional) typo. [...] > diff --git a/lisp/emacs-lisp/ert-x.el b/lisp/emacs-lisp/ert-x.el > index 83705ca5b8..7cf60bfeae 100644 > --- a/lisp/emacs-lisp/ert-x.el > +++ b/lisp/emacs-lisp/ert-x.el > @@ -496,7 +496,7 @@ ert-with-temp-file > (progn ,@body) > (ignore-errors > ,@(when buffer > - (list `(with-current-buffer buf > + (list `(with-current-buffer ,buffer > (set-buffer-modified-p nil)) > `(kill-buffer ,buffer)))) > (ignore-errors > > seems to make it go away. Thanks, fixed on emacs-29 (commit f27a330b99e). ^ permalink raw reply [flat|nested] 15+ messages in thread
* bug#60730: 29.0.60; Free variable with :buffer keyword in ert-with-temp-file 2023-01-13 1:56 ` Stefan Kangas @ 2023-01-28 14:13 ` J.P. 2023-01-28 15:03 ` Eli Zaretskii 0 siblings, 1 reply; 15+ messages in thread From: J.P. @ 2023-01-28 14:13 UTC (permalink / raw) To: 60730; +Cc: Stefan Kangas Perhaps I should have filed another report for this. It's a similar error in the same vicinity on the same branch, so I figured might as well piggyback. I'm getting "reference to free variable `utf-8'" warnings (from `elisp-flymake--batch-compile-for-flymake') when linting tests containing `ert-with-temp-file'. This doesn't show up if `coding-system-for-write' is nil or if you pass in a quoted keyword argument for `:coding'. Adding a quote like this seems to make it go away: diff --git a/lisp/emacs-lisp/ert-x.el b/lisp/emacs-lisp/ert-x.el index 98a017c8a8e..70b136c5c55 100644 --- a/lisp/emacs-lisp/ert-x.el +++ b/lisp/emacs-lisp/ert-x.el @@ -484,7 +484,7 @@ ert-with-temp-file (suffix (or suffix ert-temp-file-suffix (ert--with-temp-file-generate-suffix (or (macroexp-file-name) buffer-file-name))))) - `(let* ((coding-system-for-write ,(or coding coding-system-for-write)) + `(let* ((coding-system-for-write ',(or coding coding-system-for-write)) (,temp-file (,(if directory 'file-name-as-directory 'identity) (make-temp-file ,prefix ,directory ,suffix ,text))) (,name ,(if directory Not sure if that's the right call, though. If this keyword is already seeing action in the wild, perhaps it's worth ensuring that its argument arrives unquoted? Or maybe another type check (to accompany the one for `name') would do? Thanks in advance. ^ permalink raw reply [flat|nested] 15+ messages in thread
* bug#60730: 29.0.60; Free variable with :buffer keyword in ert-with-temp-file 2023-01-28 14:13 ` J.P. @ 2023-01-28 15:03 ` Eli Zaretskii 2023-01-28 15:56 ` J.P. 0 siblings, 1 reply; 15+ messages in thread From: Eli Zaretskii @ 2023-01-28 15:03 UTC (permalink / raw) To: J.P.; +Cc: stefankangas, 60730 > Cc: Stefan Kangas <stefankangas@gmail.com> > From: "J.P." <jp@neverwas.me> > Date: Sat, 28 Jan 2023 06:13:36 -0800 > > Perhaps I should have filed another report for this. It's a similar > error in the same vicinity on the same branch, so I figured might as > well piggyback. > > I'm getting "reference to free variable `utf-8'" warnings (from > `elisp-flymake--batch-compile-for-flymake') when linting tests > containing `ert-with-temp-file'. This doesn't show up if > `coding-system-for-write' is nil or if you pass in a quoted keyword > argument for `:coding'. Adding a quote like this seems to make it go > away: > > diff --git a/lisp/emacs-lisp/ert-x.el b/lisp/emacs-lisp/ert-x.el > index 98a017c8a8e..70b136c5c55 100644 > --- a/lisp/emacs-lisp/ert-x.el > +++ b/lisp/emacs-lisp/ert-x.el > @@ -484,7 +484,7 @@ ert-with-temp-file > (suffix (or suffix ert-temp-file-suffix > (ert--with-temp-file-generate-suffix > (or (macroexp-file-name) buffer-file-name))))) > - `(let* ((coding-system-for-write ,(or coding coding-system-for-write)) > + `(let* ((coding-system-for-write ',(or coding coding-system-for-write)) > (,temp-file (,(if directory 'file-name-as-directory 'identity) > (make-temp-file ,prefix ,directory ,suffix ,text))) > (,name ,(if directory > > Not sure if that's the right call, though. If this keyword is already > seeing action in the wild, perhaps it's worth ensuring that its argument > arrives unquoted? Or maybe another type check (to accompany the one for > `name') would do? Can you show the results of macro-expansion both when coding has a value and when it is nil (and then coding-system-for-write is nil or has a non-nil value)? ^ permalink raw reply [flat|nested] 15+ messages in thread
* bug#60730: 29.0.60; Free variable with :buffer keyword in ert-with-temp-file 2023-01-28 15:03 ` Eli Zaretskii @ 2023-01-28 15:56 ` J.P. 2023-01-28 16:13 ` Eli Zaretskii 0 siblings, 1 reply; 15+ messages in thread From: J.P. @ 2023-01-28 15:56 UTC (permalink / raw) To: Eli Zaretskii; +Cc: stefankangas, 60730 Eli Zaretskii <eliz@gnu.org> writes: >> Not sure if that's the right call, though. If this keyword is already >> seeing action in the wild, perhaps it's worth ensuring that its argument >> arrives unquoted? Or maybe another type check (to accompany the one for >> `name') would do? > > Can you show the results of macro-expansion both when coding has a > value and when it is nil (and then coding-system-for-write is nil or > has a non-nil value)? `coding-system-for-write' nil, keyword nil (ert-with-temp-file myfile :coding nil) (let* ((coding-system-for-write nil) ...) ;; keyword absent (ert-with-temp-file myfile) (let* ((coding-system-for-write nil) ...) `coding-system-for-write' nil, keyword non-nil (ert-with-temp-file myfile :coding utf-8) (let* ((coding-system-for-write utf-8) ...) ;; keyword quoted (ert-with-temp-file myfile :coding 'utf-8) (let* ((coding-system-for-write 'utf-8) ...) `coding-system-for-write' non-nil, keyword nil (setq coding-system-for-write 'utf-8) (ert-with-temp-file myfile :coding nil) (let* ((coding-system-for-write utf-8) ...) ;; keyword absent (ert-with-temp-file myfile myfile) (let* ((coding-system-for-write utf-8) ...) `coding-system-for-write' non-nil, keyword non-nil (setq coding-system-for-write 'utf-8) (ert-with-temp-file myfile :coding raw-text) (let* ((coding-system-for-write raw-text) ...) ;; keyword quoted (ert-with-temp-file myfile :coding 'raw-text) (let* ((coding-system-for-write 'raw-text) ...) ^ permalink raw reply [flat|nested] 15+ messages in thread
* bug#60730: 29.0.60; Free variable with :buffer keyword in ert-with-temp-file 2023-01-28 15:56 ` J.P. @ 2023-01-28 16:13 ` Eli Zaretskii 2023-01-29 2:00 ` J.P. 0 siblings, 1 reply; 15+ messages in thread From: Eli Zaretskii @ 2023-01-28 16:13 UTC (permalink / raw) To: J.P.; +Cc: stefankangas, 60730 > From: "J.P." <jp@neverwas.me> > Cc: 60730@debbugs.gnu.org, stefankangas@gmail.com > Date: Sat, 28 Jan 2023 07:56:22 -0800 > > Eli Zaretskii <eliz@gnu.org> writes: > > >> Not sure if that's the right call, though. If this keyword is already > >> seeing action in the wild, perhaps it's worth ensuring that its argument > >> arrives unquoted? Or maybe another type check (to accompany the one for > >> `name') would do? > > > > Can you show the results of macro-expansion both when coding has a > > value and when it is nil (and then coding-system-for-write is nil or > > has a non-nil value)? > > `coding-system-for-write' nil, keyword nil > > (ert-with-temp-file myfile :coding nil) > (let* ((coding-system-for-write nil) ...) > > ;; keyword absent > > (ert-with-temp-file myfile) > (let* ((coding-system-for-write nil) ...) > > `coding-system-for-write' nil, keyword non-nil > > (ert-with-temp-file myfile :coding utf-8) > (let* ((coding-system-for-write utf-8) ...) > > ;; keyword quoted > > (ert-with-temp-file myfile :coding 'utf-8) > (let* ((coding-system-for-write 'utf-8) ...) > > `coding-system-for-write' non-nil, keyword nil > > (setq coding-system-for-write 'utf-8) > > (ert-with-temp-file myfile :coding nil) > (let* ((coding-system-for-write utf-8) ...) > > ;; keyword absent > > (ert-with-temp-file myfile myfile) > (let* ((coding-system-for-write utf-8) ...) > > `coding-system-for-write' non-nil, keyword non-nil > > (setq coding-system-for-write 'utf-8) > > (ert-with-temp-file myfile :coding raw-text) > (let* ((coding-system-for-write raw-text) ...) > > ;; keyword quoted > > (ert-with-temp-file myfile :coding 'raw-text) > (let* ((coding-system-for-write 'raw-text) ...) Thanks, but I'm not sure I follow: coding-system's name should always be quoted, as it's a symbol. So why things like the below: (ert-with-temp-file myfile :coding raw-text) are relevant? AFAIU, they are a mistake: raw-text should be quoted, as in 'raw-text. Is the problem that a coding-system symbol is not quoted? ^ permalink raw reply [flat|nested] 15+ messages in thread
* bug#60730: 29.0.60; Free variable with :buffer keyword in ert-with-temp-file 2023-01-28 16:13 ` Eli Zaretskii @ 2023-01-29 2:00 ` J.P. 2023-01-29 4:35 ` J.P. 2023-01-29 6:28 ` Eli Zaretskii 0 siblings, 2 replies; 15+ messages in thread From: J.P. @ 2023-01-29 2:00 UTC (permalink / raw) To: Eli Zaretskii; +Cc: stefankangas, 60730 [-- Attachment #1: Type: text/plain, Size: 1047 bytes --] Eli Zaretskii <eliz@gnu.org> writes: > Thanks, but I'm not sure I follow: coding-system's name should always > be quoted, as it's a symbol. So why things like the below: > > (ert-with-temp-file myfile :coding raw-text) > > are relevant? AFAIU, they are a mistake: raw-text should be quoted, > as in 'raw-text. I shouldn't have included the keyword argument; it only muddies the waters here. The correctness of the output, what we expect to see in the expanded form, is of primary concern. > Is the problem that a coding-system symbol is not quoted? When the value of `coding-system-for-write' is non-nil, only quoting it twice survives expansion: (setq coding-system-for-write ''raw-text) (ert-with-temp-file myfile) -> (let* ((coding-system-for-write 'raw-text) ...) Otherwise, we get a free variable: (setq coding-system-for-write 'raw-text) (ert-with-temp-file myfile) -> (let* ((coding-system-for-write raw-text) ...) BTW, I'm not setting `coding-system-for-write' myself. That's being done by the diagnostic tool. [-- Attachment #2: Error --] [-- Type: image/png, Size: 36375 bytes --] ^ permalink raw reply [flat|nested] 15+ messages in thread
* bug#60730: 29.0.60; Free variable with :buffer keyword in ert-with-temp-file 2023-01-29 2:00 ` J.P. @ 2023-01-29 4:35 ` J.P. 2023-01-29 6:38 ` Eli Zaretskii 2023-01-29 6:28 ` Eli Zaretskii 1 sibling, 1 reply; 15+ messages in thread From: J.P. @ 2023-01-29 4:35 UTC (permalink / raw) To: Eli Zaretskii; +Cc: stefankangas, 60730 "J.P." <jp@neverwas.me> writes: > Eli Zaretskii <eliz@gnu.org> writes: > >> Thanks, but I'm not sure I follow: coding-system's name should always >> be quoted, as it's a symbol. So why things like the below: >> >> (ert-with-temp-file myfile :coding raw-text) >> >> are relevant? AFAIU, they are a mistake: raw-text should be quoted, >> as in 'raw-text. If, as you say, an argument to `:coding' should only ever be quoted, e.g., :coding 'raw-text then `coding' will end up quoted as well, so something like this might be enough: diff --git a/lisp/emacs-lisp/ert-x.el b/lisp/emacs-lisp/ert-x.el index 98a017c8a8e..aa02c79d32f 100644 --- a/lisp/emacs-lisp/ert-x.el +++ b/lisp/emacs-lisp/ert-x.el @@ -484,7 +484,7 @@ ert-with-temp-file (suffix (or suffix ert-temp-file-suffix (ert--with-temp-file-generate-suffix (or (macroexp-file-name) buffer-file-name))))) - `(let* ((coding-system-for-write ,(or coding coding-system-for-write)) + `(let* ((coding-system-for-write ,(or coding `',coding-system-for-write)) (,temp-file (,(if directory 'file-name-as-directory 'identity) (make-temp-file ,prefix ,directory ,suffix ,text))) (,name ,(if directory ^ permalink raw reply [flat|nested] 15+ messages in thread
* bug#60730: 29.0.60; Free variable with :buffer keyword in ert-with-temp-file 2023-01-29 4:35 ` J.P. @ 2023-01-29 6:38 ` Eli Zaretskii 2023-01-29 14:08 ` J.P. 0 siblings, 1 reply; 15+ messages in thread From: Eli Zaretskii @ 2023-01-29 6:38 UTC (permalink / raw) To: J.P.; +Cc: stefankangas, 60730 > From: "J.P." <jp@neverwas.me> > Cc: 60730@debbugs.gnu.org, stefankangas@gmail.com > Date: Sat, 28 Jan 2023 20:35:47 -0800 > > "J.P." <jp@neverwas.me> writes: > > > Eli Zaretskii <eliz@gnu.org> writes: > > > >> Thanks, but I'm not sure I follow: coding-system's name should always > >> be quoted, as it's a symbol. So why things like the below: > >> > >> (ert-with-temp-file myfile :coding raw-text) > >> > >> are relevant? AFAIU, they are a mistake: raw-text should be quoted, > >> as in 'raw-text. > > If, as you say, an argument to `:coding' should only ever be quoted, e.g., > > :coding 'raw-text > > then `coding' will end up quoted as well, so something like this might > be enough: If you say so. The `', stuff looks strange to me, but the backticks in Emacs Lisp have always been black magic. What we need to ensure that both :coding 'raw-text and :coding some-coding-variable do work as expected, including when coding-system-for-write's value is a non-nil symbol of a coding-system. Thanks. ^ permalink raw reply [flat|nested] 15+ messages in thread
* bug#60730: 29.0.60; Free variable with :buffer keyword in ert-with-temp-file 2023-01-29 6:38 ` Eli Zaretskii @ 2023-01-29 14:08 ` J.P. 2023-01-29 15:10 ` Eli Zaretskii 0 siblings, 1 reply; 15+ messages in thread From: J.P. @ 2023-01-29 14:08 UTC (permalink / raw) To: Eli Zaretskii; +Cc: stefankangas, 60730 Eli Zaretskii <eliz@gnu.org> writes: >> "J.P." <jp@neverwas.me> writes: >> >> If, as you say, an argument to `:coding' should only ever be quoted, e.g., >> >> :coding 'raw-text >> >> then `coding' will end up quoted as well, so something like this might >> be enough: > > If you say so. The `', stuff looks strange to me, but the backticks > in Emacs Lisp have always been black magic. > > What we need to ensure that both > > :coding 'raw-text > > and > > :coding some-coding-variable > > do work as expected, including when coding-system-for-write's value is > a non-nil symbol of a coding-system. Right, whatever the solution, it should cover those bases. Although, if `some-coding-variable' evaluates to nil, the change I proposed would not fall back on `coding-system-for-write'. (But perhaps it should? [1]) Also, thinking about this in earnest (for once), I'm unsure why we need to capture the value of `coding-system-for-write' at expansion time. Wouldn't it be preferable to defer evaluation until the test actually runs? IOW, when the `:coding' keyword is absent, shouldn't the final form contain -> (let* ((coding-system-for-write coding-system-for-write) ... or even -> (let* (... (meaning nothing)? If this "deferred" approach makes sense, perhaps something like this will suffice: diff --git a/lisp/emacs-lisp/ert-x.el b/lisp/emacs-lisp/ert-x.el index 98a017c8a8e..2605fc22ddf 100644 --- a/lisp/emacs-lisp/ert-x.el +++ b/lisp/emacs-lisp/ert-x.el @@ -475,7 +475,7 @@ ert-with-temp-file (:directory (setq directory (pop body))) (:text (setq text (pop body))) (:buffer (setq buffer (pop body))) - (:coding (setq coding (pop body))) + (:coding (setq coding (list (pop body)))) (_ (push keyw extra-keywords) (pop body)))) (when extra-keywords (error "Invalid keywords: %s" (mapconcat #'symbol-name extra-keywords " "))) @@ -484,7 +484,7 @@ ert-with-temp-file (suffix (or suffix ert-temp-file-suffix (ert--with-temp-file-generate-suffix (or (macroexp-file-name) buffer-file-name))))) - `(let* ((coding-system-for-write ,(or coding coding-system-for-write)) + `(let* (,@(and coding `((coding-system-for-write ,(car coding)))) (,temp-file (,(if directory 'file-name-as-directory 'identity) (make-temp-file ,prefix ,directory ,suffix ,text))) (,name ,(if directory Note that with this change, `coding-system-for-write' would only be bound when the user supplies a `:coding' argument, even if that argument is nil [2]. Anyway, if this "deferred" stuff is simply wrongheaded, please forget I ever mentioned it. Thanks. [1] If incorporating such "fallback" behavior into the "deferred" approach mentioned above is desirable, we could try something like diff --git a/lisp/emacs-lisp/ert-x.el b/lisp/emacs-lisp/ert-x.el index 98a017c8a8e..3439aacf1ab 100644 --- a/lisp/emacs-lisp/ert-x.el +++ b/lisp/emacs-lisp/ert-x.el @@ -484,7 +484,7 @@ ert-with-temp-file (suffix (or suffix ert-temp-file-suffix (ert--with-temp-file-generate-suffix (or (macroexp-file-name) buffer-file-name))))) - `(let* ((coding-system-for-write ,(or coding coding-system-for-write)) + `(let* ((coding-system-for-write (or ,coding coding-system-for-write)) (,temp-file (,(if directory 'file-name-as-directory 'identity) (make-temp-file ,prefix ,directory ,suffix ,text))) (,name ,(if directory [2] My main concern with the "fallback" route is that the user loses a rather convenient means of ignoring whatever value of `coding-system-for-write' exists in their testing environment. IOW, they cannot easily opt to favor the default selection procedure mentioned in the doc string (for `c-s-f-w'). As a user of this macro, I feel it might be handy to have the option of supplying a literal nil (or a variable evaluating to nil) to signal such intent. ^ permalink raw reply [flat|nested] 15+ messages in thread
* bug#60730: 29.0.60; Free variable with :buffer keyword in ert-with-temp-file 2023-01-29 14:08 ` J.P. @ 2023-01-29 15:10 ` Eli Zaretskii 2023-01-29 16:18 ` J.P. 0 siblings, 1 reply; 15+ messages in thread From: Eli Zaretskii @ 2023-01-29 15:10 UTC (permalink / raw) To: J.P.; +Cc: stefankangas, 60730 > From: "J.P." <jp@neverwas.me> > Cc: 60730@debbugs.gnu.org, stefankangas@gmail.com > Date: Sun, 29 Jan 2023 06:08:01 -0800 > > > What we need to ensure that both > > > > :coding 'raw-text > > > > and > > > > :coding some-coding-variable > > > > do work as expected, including when coding-system-for-write's value is > > a non-nil symbol of a coding-system. > > Right, whatever the solution, it should cover those bases. Although, if > `some-coding-variable' evaluates to nil, the change I proposed would not > fall back on `coding-system-for-write'. (But perhaps it should? [1]) Setting :coding to nil is unusual and I don't expect that to happen. Its semantics is tricky and most people aren't aware of that, so they (rightfully) don't use it. > diff --git a/lisp/emacs-lisp/ert-x.el b/lisp/emacs-lisp/ert-x.el > index 98a017c8a8e..2605fc22ddf 100644 > --- a/lisp/emacs-lisp/ert-x.el > +++ b/lisp/emacs-lisp/ert-x.el > @@ -475,7 +475,7 @@ ert-with-temp-file > (:directory (setq directory (pop body))) > (:text (setq text (pop body))) > (:buffer (setq buffer (pop body))) > - (:coding (setq coding (pop body))) > + (:coding (setq coding (list (pop body)))) > (_ (push keyw extra-keywords) (pop body)))) > (when extra-keywords > (error "Invalid keywords: %s" (mapconcat #'symbol-name extra-keywords " "))) > @@ -484,7 +484,7 @@ ert-with-temp-file > (suffix (or suffix ert-temp-file-suffix > (ert--with-temp-file-generate-suffix > (or (macroexp-file-name) buffer-file-name))))) > - `(let* ((coding-system-for-write ,(or coding coding-system-for-write)) > + `(let* (,@(and coding `((coding-system-for-write ,(car coding)))) > (,temp-file (,(if directory 'file-name-as-directory 'identity) > (make-temp-file ,prefix ,directory ,suffix ,text))) > (,name ,(if directory I don't think this is right. coding-system-for-write exists and should be heeded because it gives the user control on binding the encoding just for this single command via "C-x RET c" prefix. By contrast, the value that comes from :coding is determined by the Lisp program, and "C-x RET c" should override it. ^ permalink raw reply [flat|nested] 15+ messages in thread
* bug#60730: 29.0.60; Free variable with :buffer keyword in ert-with-temp-file 2023-01-29 15:10 ` Eli Zaretskii @ 2023-01-29 16:18 ` J.P. 0 siblings, 0 replies; 15+ messages in thread From: J.P. @ 2023-01-29 16:18 UTC (permalink / raw) To: Eli Zaretskii; +Cc: stefankangas, 60730 Eli Zaretskii <eliz@gnu.org> writes: >> From: "J.P." <jp@neverwas.me> >> Cc: 60730@debbugs.gnu.org, stefankangas@gmail.com >> Date: Sun, 29 Jan 2023 06:08:01 -0800 >> >> > What we need to ensure that both >> > >> > :coding 'raw-text >> > >> > and >> > >> > :coding some-coding-variable >> > >> > do work as expected, including when coding-system-for-write's value is >> > a non-nil symbol of a coding-system. >> >> Right, whatever the solution, it should cover those bases. Although, if >> `some-coding-variable' evaluates to nil, the change I proposed would not >> fall back on `coding-system-for-write'. (But perhaps it should? [1]) > > Setting :coding to nil is unusual and I don't expect that to happen. > Its semantics is tricky and most people aren't aware of that, so they > (rightfully) don't use it. If it's unlikely that a user would specify nil explicitly or provide a variable that might evaluate to nil, then the question of whether to fall back to `coding-system-for-write' is less important. >> diff --git a/lisp/emacs-lisp/ert-x.el b/lisp/emacs-lisp/ert-x.el >> index 98a017c8a8e..2605fc22ddf 100644 >> --- a/lisp/emacs-lisp/ert-x.el >> +++ b/lisp/emacs-lisp/ert-x.el >> @@ -475,7 +475,7 @@ ert-with-temp-file >> (:directory (setq directory (pop body))) >> (:text (setq text (pop body))) >> (:buffer (setq buffer (pop body))) >> - (:coding (setq coding (pop body))) >> + (:coding (setq coding (list (pop body)))) >> (_ (push keyw extra-keywords) (pop body)))) >> (when extra-keywords >> (error "Invalid keywords: %s" (mapconcat #'symbol-name extra-keywords " "))) >> @@ -484,7 +484,7 @@ ert-with-temp-file >> (suffix (or suffix ert-temp-file-suffix >> (ert--with-temp-file-generate-suffix >> (or (macroexp-file-name) buffer-file-name))))) >> - `(let* ((coding-system-for-write ,(or coding coding-system-for-write)) >> + `(let* (,@(and coding `((coding-system-for-write ,(car coding)))) >> (,temp-file (,(if directory 'file-name-as-directory 'identity) >> (make-temp-file ,prefix ,directory ,suffix ,text))) >> (,name ,(if directory > > I don't think this is right. coding-system-for-write exists and > should be heeded because it gives the user control on binding the > encoding just for this single command via "C-x RET c" prefix. By > contrast, the value that comes from :coding is determined by the Lisp > program, and "C-x RET c" should override it. Interesting. Makes sense to limit any damage done to that variable to the extent of the test run. Guess that should have occurred to me. So, based on what you've said, a couple remaining possibilities are diff --git a/lisp/emacs-lisp/ert-x.el b/lisp/emacs-lisp/ert-x.el index 98a017c8a8e..3439aacf1ab 100644 --- a/lisp/emacs-lisp/ert-x.el +++ b/lisp/emacs-lisp/ert-x.el @@ -484,7 +484,7 @@ ert-with-temp-file (suffix (or suffix ert-temp-file-suffix (ert--with-temp-file-generate-suffix (or (macroexp-file-name) buffer-file-name))))) - `(let* ((coding-system-for-write ,(or coding coding-system-for-write)) + `(let* ((coding-system-for-write (or ,coding coding-system-for-write)) (,temp-file (,(if directory 'file-name-as-directory 'identity) (make-temp-file ,prefix ,directory ,suffix ,text))) (,name ,(if directory and diff --git a/lisp/emacs-lisp/ert-x.el b/lisp/emacs-lisp/ert-x.el index 98a017c8a8e..3439aacf1ab 100644 --- a/lisp/emacs-lisp/ert-x.el +++ b/lisp/emacs-lisp/ert-x.el @@ -484,7 +484,7 @@ ert-with-temp-file (suffix (or suffix ert-temp-file-suffix (ert--with-temp-file-generate-suffix (or (macroexp-file-name) buffer-file-name))))) - `(let* ((coding-system-for-write ,(or coding coding-system-for-write)) + `(let* ((coding-system-for-write ,(or coding 'coding-system-for-write)) (,temp-file (,(if directory 'file-name-as-directory 'identity) (make-temp-file ,prefix ,directory ,suffix ,text))) (,name ,(if directory The bottom one doesn't fall back if `coding' is a variable that evaluates to nil. Of course, there are surely other (perhaps smarter) ways to go about this, in case anyone else wants to take a stab. ^ permalink raw reply [flat|nested] 15+ messages in thread
* bug#60730: 29.0.60; Free variable with :buffer keyword in ert-with-temp-file 2023-01-29 2:00 ` J.P. 2023-01-29 4:35 ` J.P. @ 2023-01-29 6:28 ` Eli Zaretskii 2023-01-29 9:56 ` Andreas Schwab 1 sibling, 1 reply; 15+ messages in thread From: Eli Zaretskii @ 2023-01-29 6:28 UTC (permalink / raw) To: J.P.; +Cc: stefankangas, 60730 > From: "J.P." <jp@neverwas.me> > Cc: 60730@debbugs.gnu.org, stefankangas@gmail.com > Date: Sat, 28 Jan 2023 18:00:40 -0800 > > When the value of `coding-system-for-write' is non-nil, only quoting it > twice survives expansion: > > (setq coding-system-for-write ''raw-text) > (ert-with-temp-file myfile) > > -> (let* ((coding-system-for-write 'raw-text) ...) > > Otherwise, we get a free variable: > > (setq coding-system-for-write 'raw-text) > (ert-with-temp-file myfile) > > -> (let* ((coding-system-for-write raw-text) ...) OK, in that case we should indeed solve this in the code by something like the patch you proposed. ^ permalink raw reply [flat|nested] 15+ messages in thread
* bug#60730: 29.0.60; Free variable with :buffer keyword in ert-with-temp-file 2023-01-29 6:28 ` Eli Zaretskii @ 2023-01-29 9:56 ` Andreas Schwab 2023-01-29 10:29 ` Eli Zaretskii 0 siblings, 1 reply; 15+ messages in thread From: Andreas Schwab @ 2023-01-29 9:56 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 60730, stefankangas, J.P. On Jan 29 2023, Eli Zaretskii wrote: >> From: "J.P." <jp@neverwas.me> >> Cc: 60730@debbugs.gnu.org, stefankangas@gmail.com >> Date: Sat, 28 Jan 2023 18:00:40 -0800 >> >> When the value of `coding-system-for-write' is non-nil, only quoting it >> twice survives expansion: >> >> (setq coding-system-for-write ''raw-text) >> (ert-with-temp-file myfile) >> >> -> (let* ((coding-system-for-write 'raw-text) ...) >> >> Otherwise, we get a free variable: >> >> (setq coding-system-for-write 'raw-text) >> (ert-with-temp-file myfile) >> >> -> (let* ((coding-system-for-write raw-text) ...) > > OK, in that case we should indeed solve this in the code by something > like the patch you proposed. I am surprised that the commit worked for you when you installed it. Do you perhaps have defined a global variable undecided-unix in your Emacs instance? -- Andreas Schwab, schwab@linux-m68k.org GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510 2552 DF73 E780 A9DA AEC1 "And now for something completely different." ^ permalink raw reply [flat|nested] 15+ messages in thread
* bug#60730: 29.0.60; Free variable with :buffer keyword in ert-with-temp-file 2023-01-29 9:56 ` Andreas Schwab @ 2023-01-29 10:29 ` Eli Zaretskii 0 siblings, 0 replies; 15+ messages in thread From: Eli Zaretskii @ 2023-01-29 10:29 UTC (permalink / raw) To: Andreas Schwab; +Cc: 60730, stefankangas, jp > From: Andreas Schwab <schwab@linux-m68k.org> > Cc: "J.P." <jp@neverwas.me>, stefankangas@gmail.com, 60730@debbugs.gnu.org > Date: Sun, 29 Jan 2023 10:56:08 +0100 > > On Jan 29 2023, Eli Zaretskii wrote: > > >> From: "J.P." <jp@neverwas.me> > >> Cc: 60730@debbugs.gnu.org, stefankangas@gmail.com > >> Date: Sat, 28 Jan 2023 18:00:40 -0800 > >> > >> When the value of `coding-system-for-write' is non-nil, only quoting it > >> twice survives expansion: > >> > >> (setq coding-system-for-write ''raw-text) > >> (ert-with-temp-file myfile) > >> > >> -> (let* ((coding-system-for-write 'raw-text) ...) > >> > >> Otherwise, we get a free variable: > >> > >> (setq coding-system-for-write 'raw-text) > >> (ert-with-temp-file myfile) > >> > >> -> (let* ((coding-system-for-write raw-text) ...) > > > > OK, in that case we should indeed solve this in the code by something > > like the patch you proposed. > > I am surprised that the commit worked for you when you installed it. Do > you perhaps have defined a global variable undecided-unix in your Emacs > instance? No. ^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2023-01-29 16:18 UTC | newest] Thread overview: 15+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-01-11 13:50 bug#60730: 29.0.60; Free variable with :buffer keyword in ert-with-temp-file J.P. 2023-01-13 1:56 ` Stefan Kangas 2023-01-28 14:13 ` J.P. 2023-01-28 15:03 ` Eli Zaretskii 2023-01-28 15:56 ` J.P. 2023-01-28 16:13 ` Eli Zaretskii 2023-01-29 2:00 ` J.P. 2023-01-29 4:35 ` J.P. 2023-01-29 6:38 ` Eli Zaretskii 2023-01-29 14:08 ` J.P. 2023-01-29 15:10 ` Eli Zaretskii 2023-01-29 16:18 ` J.P. 2023-01-29 6:28 ` Eli Zaretskii 2023-01-29 9:56 ` Andreas Schwab 2023-01-29 10:29 ` Eli Zaretskii
Code repositories for project(s) associated with this public inbox https://git.savannah.gnu.org/cgit/emacs.git This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).
![[-- Attachment #2: Error --] [-- Type: image/png, Size: 36375 bytes --]](../../874jsaumk7.fsf@neverwas.me/2-flymake-ert-with-temp-file.png){kind=link}