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
* 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 external index
https://git.savannah.gnu.org/cgit/emacs.git
https://git.savannah.gnu.org/cgit/emacs/org-mode.git
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.