* 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 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 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: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
* 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
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).