From: "J.P." <jp@neverwas.me>
To: Eli Zaretskii <eliz@gnu.org>
Cc: stefankangas@gmail.com, 60730@debbugs.gnu.org
Subject: bug#60730: 29.0.60; Free variable with :buffer keyword in ert-with-temp-file
Date: Sun, 29 Jan 2023 08:18:48 -0800 [thread overview]
Message-ID: <87sfftjouv.fsf@neverwas.me> (raw)
In-Reply-To: <83pmax74wy.fsf@gnu.org> (Eli Zaretskii's message of "Sun, 29 Jan 2023 17:10:21 +0200")
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.
next prev parent reply other threads:[~2023-01-29 16:18 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
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. [this message]
2023-01-29 6:28 ` Eli Zaretskii
2023-01-29 9:56 ` Andreas Schwab
2023-01-29 10:29 ` Eli Zaretskii
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: https://www.gnu.org/software/emacs/
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87sfftjouv.fsf@neverwas.me \
--to=jp@neverwas.me \
--cc=60730@debbugs.gnu.org \
--cc=eliz@gnu.org \
--cc=stefankangas@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).