GNU bug report logs - #60730
29.0.60; Free variable with :buffer keyword in ert-with-temp-file

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 60730 in the body.
You can then email your comments to 60730 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: "J.P." <jp <at> neverwas.me>
To: bug-gnu-emacs <at> gnu.org
Cc: Stefan Kangas <stefankangas <at> gmail.com>
Subject: 29.0.60; Free variable with :buffer keyword in ert-with-temp-file
Date: Wed, 11 Jan 2023 05:50:17 -0800

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))

Message #10 received at 60730-done <at> debbugs.gnu.org (full text, mbox):

From: Stefan Kangas <stefankangas <at> gmail.com>
To: "J.P." <jp <at> neverwas.me>
Cc: 60730-done <at> debbugs.gnu.org
Subject: Re: bug#60730: 29.0.60;
 Free variable with :buffer keyword in ert-with-temp-file
Date: Thu, 12 Jan 2023 17:56:33 -0800

"J.P." <jp <at> 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).

Message #13 received at 60730 <at> debbugs.gnu.org (full text, mbox):

From: "J.P." <jp <at> neverwas.me>
To: 60730 <at> debbugs.gnu.org
Cc: Stefan Kangas <stefankangas <at> gmail.com>
Subject: Re: bug#60730: 29.0.60; Free variable with :buffer keyword in
 ert-with-temp-file
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?

Thanks in advance.

Message #16 received at 60730 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: "J.P." <jp <at> neverwas.me>
Cc: stefankangas <at> gmail.com, 60730 <at> debbugs.gnu.org
Subject: Re: bug#60730: 29.0.60;
 Free variable with :buffer keyword in ert-with-temp-file
Date: Sat, 28 Jan 2023 17:03:07 +0200

> Cc: Stefan Kangas <stefankangas <at> gmail.com>
> From: "J.P." <jp <at> 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)?

Message #19 received at 60730 <at> debbugs.gnu.org (full text, mbox):

From: "J.P." <jp <at> neverwas.me>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: stefankangas <at> gmail.com, 60730 <at> debbugs.gnu.org
Subject: Re: bug#60730: 29.0.60; Free variable with :buffer keyword in
 ert-with-temp-file
Date: Sat, 28 Jan 2023 07:56:22 -0800

Eli Zaretskii <eliz <at> 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) ...)

Message #22 received at 60730 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: "J.P." <jp <at> neverwas.me>
Cc: stefankangas <at> gmail.com, 60730 <at> debbugs.gnu.org
Subject: Re: bug#60730: 29.0.60; Free variable with :buffer keyword in
 ert-with-temp-file
Date: Sat, 28 Jan 2023 18:13:20 +0200

> From: "J.P." <jp <at> neverwas.me>
> Cc: 60730 <at> debbugs.gnu.org,  stefankangas <at> gmail.com
> Date: Sat, 28 Jan 2023 07:56:22 -0800
> 
> Eli Zaretskii <eliz <at> 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?

Message #25 received at 60730 <at> debbugs.gnu.org (full text, mbox):

From: "J.P." <jp <at> neverwas.me>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: stefankangas <at> gmail.com, 60730 <at> debbugs.gnu.org
Subject: Re: bug#60730: 29.0.60; Free variable with :buffer keyword in
 ert-with-temp-file
Date: Sat, 28 Jan 2023 18:00:40 -0800

[Message part 1 (text/plain, inline)]

Eli Zaretskii <eliz <at> 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.

[flymake-ert-with-temp-file.png (image/png, attachment)]

Message #28 received at 60730 <at> debbugs.gnu.org (full text, mbox):

From: "J.P." <jp <at> neverwas.me>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: stefankangas <at> gmail.com, 60730 <at> debbugs.gnu.org
Subject: Re: bug#60730: 29.0.60; Free variable with :buffer keyword in
 ert-with-temp-file
Date: Sat, 28 Jan 2023 20:35:47 -0800

"J.P." <jp <at> neverwas.me> writes:

> Eli Zaretskii <eliz <at> 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

Message #31 received at 60730 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: "J.P." <jp <at> neverwas.me>
Cc: stefankangas <at> gmail.com, 60730 <at> debbugs.gnu.org
Subject: Re: bug#60730: 29.0.60; Free variable with :buffer keyword in
 ert-with-temp-file
Date: Sun, 29 Jan 2023 08:28:57 +0200

> From: "J.P." <jp <at> neverwas.me>
> Cc: 60730 <at> debbugs.gnu.org,  stefankangas <at> 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.

Message #34 received at 60730 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: "J.P." <jp <at> neverwas.me>
Cc: stefankangas <at> gmail.com, 60730 <at> debbugs.gnu.org
Subject: Re: bug#60730: 29.0.60; Free variable with :buffer keyword in
 ert-with-temp-file
Date: Sun, 29 Jan 2023 08:38:22 +0200

> From: "J.P." <jp <at> neverwas.me>
> Cc: 60730 <at> debbugs.gnu.org,  stefankangas <at> gmail.com
> Date: Sat, 28 Jan 2023 20:35:47 -0800
> 
> "J.P." <jp <at> neverwas.me> writes:
> 
> > Eli Zaretskii <eliz <at> 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.

Message #37 received at 60730 <at> debbugs.gnu.org (full text, mbox):

From: Andreas Schwab <schwab <at> linux-m68k.org>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 60730 <at> debbugs.gnu.org, stefankangas <at> gmail.com, "J.P." <jp <at> neverwas.me>
Subject: Re: bug#60730: 29.0.60; Free variable with :buffer keyword in
 ert-with-temp-file
Date: Sun, 29 Jan 2023 10:56:08 +0100

On Jan 29 2023, Eli Zaretskii wrote:

>> From: "J.P." <jp <at> neverwas.me>
>> Cc: 60730 <at> debbugs.gnu.org,  stefankangas <at> 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 <at> linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

Message #40 received at 60730 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Andreas Schwab <schwab <at> linux-m68k.org>
Cc: 60730 <at> debbugs.gnu.org, stefankangas <at> gmail.com, jp <at> neverwas.me
Subject: Re: bug#60730: 29.0.60; Free variable with :buffer keyword in
 ert-with-temp-file
Date: Sun, 29 Jan 2023 12:29:31 +0200

> From: Andreas Schwab <schwab <at> linux-m68k.org>
> Cc: "J.P." <jp <at> neverwas.me>,  stefankangas <at> gmail.com,  60730 <at> debbugs.gnu.org
> Date: Sun, 29 Jan 2023 10:56:08 +0100
> 
> On Jan 29 2023, Eli Zaretskii wrote:
> 
> >> From: "J.P." <jp <at> neverwas.me>
> >> Cc: 60730 <at> debbugs.gnu.org,  stefankangas <at> 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.

Message #43 received at 60730 <at> debbugs.gnu.org (full text, mbox):

From: "J.P." <jp <at> neverwas.me>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: stefankangas <at> gmail.com, 60730 <at> debbugs.gnu.org
Subject: Re: bug#60730: 29.0.60; Free variable with :buffer keyword in
 ert-with-temp-file
Date: Sun, 29 Jan 2023 06:08:01 -0800

Eli Zaretskii <eliz <at> gnu.org> writes:

>> "J.P." <jp <at> 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.

Message #46 received at 60730 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: "J.P." <jp <at> neverwas.me>
Cc: stefankangas <at> gmail.com, 60730 <at> debbugs.gnu.org
Subject: Re: bug#60730: 29.0.60; Free variable with :buffer keyword in
 ert-with-temp-file
Date: Sun, 29 Jan 2023 17:10:21 +0200

> From: "J.P." <jp <at> neverwas.me>
> Cc: 60730 <at> debbugs.gnu.org,  stefankangas <at> 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.

Message #49 received at 60730 <at> debbugs.gnu.org (full text, mbox):

From: "J.P." <jp <at> neverwas.me>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: stefankangas <at> gmail.com, 60730 <at> debbugs.gnu.org
Subject: Re: bug#60730: 29.0.60; Free variable with :buffer keyword in
 ert-with-temp-file
Date: Sun, 29 Jan 2023 08:18:48 -0800

Eli Zaretskii <eliz <at> gnu.org> writes:

>> From: "J.P." <jp <at> neverwas.me>
>> Cc: 60730 <at> debbugs.gnu.org,  stefankangas <at> 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.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.