GNU bug report logs - #63513
[PATCH] Make persist-defvar work with records and hash tables

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 63513 in the body.
You can then email your comments to 63513 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: Joseph Turner <joseph <at> breatheoutbreathe.in>
To: bug-gnu-emacs <at> gnu.org
Cc: Adam Porter <adam <at> alphapapa.net>
Subject: [PATCH] Make persist-defvar work with records and hash tables
Date: Sun, 14 May 2023 22:56:20 -0700

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

Hello!

persist-defvar does not persist records or hash tables correctly.

Here's a minimal example with a hash table:

(progn
  (persist-defvar foo (make-hash-table :test #'equal) "docstring")
  (puthash 'bar t foo)
  (persist-save 'foo)
  (gethash 'bar (persist-default 'foo))) ;; Expected nil, got t

This patch fixes persisting records by using copy-tree. Currently,
copy-tree does not work with records (see
<https://lists.gnu.org/archive/html/bug-gnu-emacs/2023-05/msg00875.html>).
The following snippet will return the expected value only when both this
patch and the above-linked patch are applied:

(progn
  (cl-defstruct baz a)
  (persist-defvar quux (make-baz :a nil) "docstring")
  (setf (baz-a quux) t)
  (persist-save 'quux)
  (baz-a (persist-default 'quux))) ;; Expected nil, got t

Before applying this patch, the updated values in both cases are not
persisted to disk. With the patch, the updated values are persisted as
expected.

Best,

Joseph

[0001-Make-persist-defvar-work-with-records-and-hash-table.patch (text/x-diff, attachment)]

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Joseph Turner <joseph <at> breatheoutbreathe.in>,
 Phillip Lord <phillip.lord <at> russet.org.uk>,
 Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: adam <at> alphapapa.net, 63513 <at> debbugs.gnu.org
Subject: Re: bug#63513: [PATCH] Make persist-defvar work with records and hash
 tables
Date: Mon, 15 May 2023 14:31:40 +0300

> Cc: Adam Porter <adam <at> alphapapa.net>
> Date: Sun, 14 May 2023 22:56:20 -0700
> From:  Joseph Turner via "Bug reports for GNU Emacs,
>  the Swiss army knife of text editors" <bug-gnu-emacs <at> gnu.org>
> 
> persist-defvar does not persist records or hash tables correctly.
> 
> Here's a minimal example with a hash table:
> 
> (progn
>   (persist-defvar foo (make-hash-table :test #'equal) "docstring")
>   (puthash 'bar t foo)
>   (persist-save 'foo)
>   (gethash 'bar (persist-default 'foo))) ;; Expected nil, got t
> 
> This patch fixes persisting records by using copy-tree. Currently,
> copy-tree does not work with records (see
> <https://lists.gnu.org/archive/html/bug-gnu-emacs/2023-05/msg00875.html>).
> The following snippet will return the expected value only when both this
> patch and the above-linked patch are applied:
> 
> (progn
>   (cl-defstruct baz a)
>   (persist-defvar quux (make-baz :a nil) "docstring")
>   (setf (baz-a quux) t)
>   (persist-save 'quux)
>   (baz-a (persist-default 'quux))) ;; Expected nil, got t
> 
> Before applying this patch, the updated values in both cases are not
> persisted to disk. With the patch, the updated values are persisted as
> expected.

Philip, any comments?

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

From: Joseph Turner <joseph <at> breatheoutbreathe.in>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: adam <at> alphapapa.net, 63513 <at> debbugs.gnu.org,
 Stefan Monnier <monnier <at> iro.umontreal.ca>,
 Phillip Lord <phillip.lord <at> russet.org.uk>
Subject: Re: bug#63513: [PATCH] Make persist-defvar work with records and
 hash tables
Date: Tue, 23 May 2023 13:14:29 -0700

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

The patch should now work on Emacs versions before Emacs 30. I also
added tests for persisting records and hash tables.

Instead of copying the updated behavior of copy-tree into persist.el,
would it be more appropriate to require compat.el?

Best,

Joseph

[0001-Add-test-persist-save-macro.patch (text/x-diff, attachment)]

[0002-Add-persist-hash-equal.patch (text/x-diff, attachment)]

[0003-Make-persist-defvar-work-with-hash-tables.patch (text/x-diff, attachment)]

[0004-Add-persist-copy-tree.patch (text/x-diff, attachment)]

[0005-Make-persist-defvar-work-with-records.patch (text/x-diff, attachment)]

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

From: Joseph Turner <joseph <at> breatheoutbreathe.in>
To: Eli Zaretskii <eliz <at> gnu.org>, Phillip Lord <phillip.lord <at> russet.org.uk>,
 Stefan Monnier <monnier <at> iro.umontreal.ca>, 63513 <at> debbugs.gnu.org,
 adam <at> alphapapa.net
Subject: Re: bug#63513: [PATCH] Make persist-defvar work with records and
 hash tables
Date: Sat, 02 Sep 2023 16:54:00 -0700

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

I fixed the definition of persist-hash-equal, and rebased.

[0001-Add-test-persist-save-macro.patch (text/x-diff, attachment)]

[0002-Add-persist-hash-equal.patch (text/x-diff, attachment)]

[0003-Make-persist-defvar-work-with-hash-tables.patch (text/x-diff, attachment)]

[0004-Add-persist-copy-tree.patch (text/x-diff, attachment)]

[0005-Make-persist-defvar-work-with-records.patch (text/x-diff, attachment)]

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Joseph Turner <joseph <at> breatheoutbreathe.in>
Cc: adam <at> alphapapa.net, 63513 <at> debbugs.gnu.org, monnier <at> iro.umontreal.ca,
 phillip.lord <at> russet.org.uk
Subject: Re: bug#63513: [PATCH] Make persist-defvar work with records and
 hash tables
Date: Sun, 03 Sep 2023 09:08:09 +0300

> From: Joseph Turner <joseph <at> breatheoutbreathe.in>
> Date: Sat, 02 Sep 2023 16:54:00 -0700
> 
> I fixed the definition of persist-hash-equal, and rebased.

Thanks, but this lacks the commit log messages (see CONTRIBUTE).

Also, I think the new features warrant a NEWS entry.

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

From: Joseph Turner <joseph <at> breatheoutbreathe.in>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: Adam Porter <adam.porter <at> 47ap.net>, 63513 <at> debbugs.gnu.org,
 monnier <at> iro.umontreal.ca, phillip.lord <at> russet.org.uk
Subject: Re: bug#63513: [PATCH] Make persist-defvar work with records and
 hash tables
Date: Sun, 03 Sep 2023 17:29:22 -0700

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

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

> Thanks, but this lacks the commit log messages (see CONTRIBUTE).

I have added commit log messages. See attached patches.

> Also, I think the new features warrant a NEWS entry.

Should that go in the NEWS file in the main Emacs repo? I don't see a
NEWS file in either of the main or externals/persist ELPA branches.

Joseph

[0001-Add-test-persist-save-macro.patch (text/x-diff, attachment)]

[0002-Add-persist-equal.patch (text/x-diff, attachment)]

[0003-Make-persist-defvar-work-with-hash-tables.patch (text/x-diff, attachment)]

[0004-Add-persist-copy-tree.patch (text/x-diff, attachment)]

[0005-Make-persist-defvar-work-with-records.patch (text/x-diff, attachment)]

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Joseph Turner <joseph <at> breatheoutbreathe.in>
Cc: adam.porter <at> 47ap.net, 63513 <at> debbugs.gnu.org, monnier <at> iro.umontreal.ca,
 phillip.lord <at> russet.org.uk
Subject: Re: bug#63513: [PATCH] Make persist-defvar work with records and
 hash tables
Date: Mon, 04 Sep 2023 14:33:55 +0300

> From: Joseph Turner <joseph <at> breatheoutbreathe.in>
> Cc: phillip.lord <at> russet.org.uk, monnier <at> iro.umontreal.ca,
>  63513 <at> debbugs.gnu.org, Adam Porter <adam.porter <at> 47ap.net>
> Date: Sun, 03 Sep 2023 17:29:22 -0700
> 
> > Also, I think the new features warrant a NEWS entry.
> 
> Should that go in the NEWS file in the main Emacs repo? I don't see a
> NEWS file in either of the main or externals/persist ELPA branches.

I'm terribly sorry: I haven't realized this is for ELPA, I thought it
was for emacs.git.  My bad Please ignore what I wrote in my previous
message.

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

From: Joseph Turner <joseph <at> breatheoutbreathe.in>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: adam.porter <at> 47ap.net, 63513 <at> debbugs.gnu.org, monnier <at> iro.umontreal.ca,
 phillip.lord <at> russet.org.uk
Subject: Re: bug#63513: [PATCH] Make persist-defvar work with records and hash tables
Date: Mon, 04 Sep 2023 08:57:13 -0700

On September 4, 2023 4:33:55 AM PDT, Eli Zaretskii <eliz <at> gnu.org> wrote:
>> From: Joseph Turner <joseph <at> breatheoutbreathe.in>
>> Cc: phillip.lord <at> russet.org.uk, monnier <at> iro.umontreal.ca,
>>  63513 <at> debbugs.gnu.org, Adam Porter <adam.porter <at> 47ap.net>
>> Date: Sun, 03 Sep 2023 17:29:22 -0700
>> 
>> > Also, I think the new features warrant a NEWS entry.
>> 
>> Should that go in the NEWS file in the main Emacs repo? I don't see a
>> NEWS file in either of the main or externals/persist ELPA branches.
>
>I'm terribly sorry: I haven't realized this is for ELPA, I thought it
>was for emacs.git.  My bad Please ignore what I wrote in my previous
>message.

No problem! It gave me a chance to do a final editing pass. What's the next step? I would be glad for Phillip's review, if possible.

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

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Joseph Turner <joseph <at> breatheoutbreathe.in>
Cc: adam.porter <at> 47ap.net, Eli Zaretskii <eliz <at> gnu.org>, 63513 <at> debbugs.gnu.org,
 phillip.lord <at> russet.org.uk
Subject: Re: bug#63513: [PATCH] Make persist-defvar work with records and
 hash tables
Date: Mon, 04 Sep 2023 13:05:13 -0400

Joseph Turner [2023-09-04 08:57:13] wrote:
> On September 4, 2023 4:33:55 AM PDT, Eli Zaretskii <eliz <at> gnu.org> wrote:
>>> From: Joseph Turner <joseph <at> breatheoutbreathe.in>
>>> Cc: phillip.lord <at> russet.org.uk, monnier <at> iro.umontreal.ca,
>>>  63513 <at> debbugs.gnu.org, Adam Porter <adam.porter <at> 47ap.net>
>>> Date: Sun, 03 Sep 2023 17:29:22 -0700
>>> 
>>> > Also, I think the new features warrant a NEWS entry.
>>> 
>>> Should that go in the NEWS file in the main Emacs repo? I don't see a
>>> NEWS file in either of the main or externals/persist ELPA branches.
>>
>>I'm terribly sorry: I haven't realized this is for ELPA, I thought it
>>was for emacs.git.  My bad Please ignore what I wrote in my previous
>>message.
>
> No problem! It gave me a chance to do a final editing pass. What's the next
> step? I would be glad for Phillip's review, if possible.

I was about to ping Phil over on https://gitlab.com/phillord/persist/
but was reminded along the way that Phil gave me write access to
that repository.

Could you send me the result of your final editing pass?


        Stefan

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

From: Joseph Turner <joseph <at> breatheoutbreathe.in>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: Adam Porter <adam <at> alphapapa.net>, Eli Zaretskii <eliz <at> gnu.org>,
 63513 <at> debbugs.gnu.org, phillip.lord <at> russet.org.uk
Subject: Re: bug#63513: [PATCH] Make persist-defvar work with records and
 hash tables
Date: Mon, 04 Sep 2023 15:28:15 -0700

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

Stefan Monnier <monnier <at> iro.umontreal.ca> writes:
> I was about to ping Phil over on https://gitlab.com/phillord/persist/
> but was reminded along the way that Phil gave me write access to
> that repository.
>
> Could you send me the result of your final editing pass?

Here you go!

[0001-Add-test-persist-save-macro.patch (text/x-diff, attachment)]

[0002-Add-persist-equal.patch (text/x-diff, attachment)]

[0003-Make-persist-defvar-work-with-hash-tables.patch (text/x-diff, attachment)]

[0004-Add-persist-copy-tree.patch (text/x-diff, attachment)]

[0005-Make-persist-defvar-work-with-records.patch (text/x-diff, attachment)]

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

From: phillip.lord <at> russet.org.uk
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: adam.porter <at> 47ap.net, Eli Zaretskii <eliz <at> gnu.org>, 63513 <at> debbugs.gnu.org,
 Joseph Turner <joseph <at> breatheoutbreathe.in>
Subject: Re: bug#63513: [PATCH] Make persist-defvar work with records and hash
 tables
Date: Tue, 05 Sep 2023 16:08:11 +0100

On 2023-09-04 18:05, Stefan Monnier wrote:
> Joseph Turner [2023-09-04 08:57:13] wrote:
>> On September 4, 2023 4:33:55 AM PDT, Eli Zaretskii <eliz <at> gnu.org> 
>> wrote:
>>>> From: Joseph Turner <joseph <at> breatheoutbreathe.in>
>>>> Cc: phillip.lord <at> russet.org.uk, monnier <at> iro.umontreal.ca,
>>>>  63513 <at> debbugs.gnu.org, Adam Porter <adam.porter <at> 47ap.net>
>>>> Date: Sun, 03 Sep 2023 17:29:22 -0700
>>>> 
>>>> > Also, I think the new features warrant a NEWS entry.
>>>> 
>>>> Should that go in the NEWS file in the main Emacs repo? I don't see 
>>>> a
>>>> NEWS file in either of the main or externals/persist ELPA branches.
>>> 
>>> I'm terribly sorry: I haven't realized this is for ELPA, I thought it
>>> was for emacs.git.  My bad Please ignore what I wrote in my previous
>>> message.
>> 
>> No problem! It gave me a chance to do a final editing pass. What's the 
>> next
>> step? I would be glad for Phillip's review, if possible.
> 
> I was about to ping Phil over on https://gitlab.com/phillord/persist/
> but was reminded along the way that Phil gave me write access to
> that repository.
> 
> Could you send me the result of your final editing pass?


Indeed! I am afraid I am fairly unresponsive these days ("real life" 
intrudes), but I am more than happy for you to update there.

Phil

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

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Joseph Turner <joseph <at> breatheoutbreathe.in>
Cc: Adam Porter <adam <at> alphapapa.net>, Eli Zaretskii <eliz <at> gnu.org>,
 phillip.lord <at> russet.org.uk, 63513-done <at> debbugs.gnu.org
Subject: Re: bug#63513: [PATCH] Make persist-defvar work with records and
 hash tables
Date: Tue, 05 Sep 2023 17:06:17 -0400

> Here you go!

Thanks, pushed!


        Stefan

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

From: Ihor Radchenko <yantar92 <at> posteo.net>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: Adam Porter <adam <at> alphapapa.net>, Eli Zaretskii <eliz <at> gnu.org>,
 phillip.lord <at> russet.org.uk, 63513-done <at> debbugs.gnu.org,
 Joseph Turner <joseph <at> breatheoutbreathe.in>
Subject: Re: bug#63513: [PATCH] Make persist-defvar work with records and
 hash tables
Date: Fri, 08 Sep 2023 11:30:52 +0000

Stefan Monnier via "Bug reports for GNU Emacs, the Swiss army knife of
text editors" <bug-gnu-emacs <at> gnu.org> writes:

>> Here you go!
>
> Thanks, pushed!

May it be possible to promote `persist-hash-equal' and
`persist-copy-tree' to common subr.el functions?

The topic of comparing hash tables has been previously discussed in
https://yhetil.org/emacs-devel/871qvz4kdw.fsf <at> localhost/

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Ihor Radchenko <yantar92 <at> posteo.net>
Cc: adam <at> alphapapa.net, phillip.lord <at> russet.org.uk, 63513-done <at> debbugs.gnu.org,
 monnier <at> iro.umontreal.ca, joseph <at> breatheoutbreathe.in
Subject: Re: bug#63513: [PATCH] Make persist-defvar work with records and
 hash tables
Date: Fri, 08 Sep 2023 14:58:37 +0300

> From: Ihor Radchenko <yantar92 <at> posteo.net>
> Cc: Joseph Turner <joseph <at> breatheoutbreathe.in>, Adam Porter
>  <adam <at> alphapapa.net>, Eli Zaretskii <eliz <at> gnu.org>,
>  phillip.lord <at> russet.org.uk, 63513-done <at> debbugs.gnu.org
> Date: Fri, 08 Sep 2023 11:30:52 +0000
> 
> May it be possible to promote `persist-hash-equal' and
> `persist-copy-tree' to common subr.el functions?

Why do we need them in subr.el, i.e. preloaded?  Why cannot these
functions be autoloaded instead?

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

From: Ihor Radchenko <yantar92 <at> posteo.net>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: adam <at> alphapapa.net, phillip.lord <at> russet.org.uk, 63513-done <at> debbugs.gnu.org,
 monnier <at> iro.umontreal.ca, joseph <at> breatheoutbreathe.in
Subject: Re: bug#63513: [PATCH] Make persist-defvar work with records and
 hash tables
Date: Fri, 08 Sep 2023 12:06:06 +0000

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

>> May it be possible to promote `persist-hash-equal' and
>> `persist-copy-tree' to common subr.el functions?
>
> Why do we need them in subr.el, i.e. preloaded?  Why cannot these
> functions be autoloaded instead?

Similar functions - `string-equal-ignore-case' and `copy-tree' - are
already in subr.el. I do not see why the new functions discussed here
should be any different.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Ihor Radchenko <yantar92 <at> posteo.net>
Cc: adam <at> alphapapa.net, phillip.lord <at> russet.org.uk, 63513-done <at> debbugs.gnu.org,
 monnier <at> iro.umontreal.ca, joseph <at> breatheoutbreathe.in
Subject: Re: bug#63513: [PATCH] Make persist-defvar work with records and
 hash tables
Date: Fri, 08 Sep 2023 15:46:14 +0300

> From: Ihor Radchenko <yantar92 <at> posteo.net>
> Cc: monnier <at> iro.umontreal.ca, joseph <at> breatheoutbreathe.in,
>  adam <at> alphapapa.net, phillip.lord <at> russet.org.uk, 63513-done <at> debbugs.gnu.org
> Date: Fri, 08 Sep 2023 12:06:06 +0000
> 
> Eli Zaretskii <eliz <at> gnu.org> writes:
> 
> >> May it be possible to promote `persist-hash-equal' and
> >> `persist-copy-tree' to common subr.el functions?
> >
> > Why do we need them in subr.el, i.e. preloaded?  Why cannot these
> > functions be autoloaded instead?
> 
> Similar functions - `string-equal-ignore-case' and `copy-tree' - are
> already in subr.el. I do not see why the new functions discussed here
> should be any different.

We make the decision whether a function must be preloaded in a case by
case basis, to avoid making the memory footprint of an Emacs session
larger than it needs to be.  So arguments "by similarity" are not
useful in this case; you need to explain why you think these functions
are needed right from the startup.  Valid reasons include, among
others:

  . function is used by the startup code or during dumping
  . function is used by all or many important configurations
    immediately after startup

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

From: Ihor Radchenko <yantar92 <at> posteo.net>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: adam <at> alphapapa.net, phillip.lord <at> russet.org.uk, 63513-done <at> debbugs.gnu.org,
 monnier <at> iro.umontreal.ca, joseph <at> breatheoutbreathe.in
Subject: Re: bug#63513: [PATCH] Make persist-defvar work with records and
 hash tables
Date: Fri, 08 Sep 2023 12:51:41 +0000

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

>> > Why do we need them in subr.el, i.e. preloaded?  Why cannot these
>> > functions be autoloaded instead?
>> 
>> Similar functions - `string-equal-ignore-case' and `copy-tree' - are
>> already in subr.el. I do not see why the new functions discussed here
>> should be any different.
>
> We make the decision whether a function must be preloaded in a case by
> case basis, to avoid making the memory footprint of an Emacs session
> larger than it needs to be.  So arguments "by similarity" are not
> useful in this case; you need to explain why you think these functions
> are needed right from the startup.
> ...

Got it. Then, I do not insist on subr.el specifically. Autoloading is
fine.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>

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

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Ihor Radchenko <yantar92 <at> posteo.net>
Cc: Adam Porter <adam <at> alphapapa.net>, Eli Zaretskii <eliz <at> gnu.org>,
 phillip.lord <at> russet.org.uk, 63513-done <at> debbugs.gnu.org,
 Joseph Turner <joseph <at> breatheoutbreathe.in>
Subject: Re: bug#63513: [PATCH] Make persist-defvar work with records and
 hash tables
Date: Fri, 08 Sep 2023 12:36:35 -0400

>>> Here you go!
>> Thanks, pushed!
> May it be possible to promote `persist-hash-equal' and
> `persist-copy-tree' to common subr.el functions?
> The topic of comparing hash tables has been previously discussed in
> https://yhetil.org/emacs-devel/871qvz4kdw.fsf <at> localhost/

Hmm... AFAICT `persist-copy-tree` is a copy of our current `copy-tree`,
so there's nothing to do there :-)

As for `persist-hash-equal` (which is actually called `persist-equal`,
AFAICT), I think if we want to move it out of `persist.el` we need to
improve it so it also looks inside hash-tables that occur within
vectors, conses, etc...

Maybe, like Sam Steingold suggested, we should simply consider the
current treatment of `equal` on hash-tables to be a bug.


        Stefan

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

From: Ihor Radchenko <yantar92 <at> posteo.net>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: Adam Porter <adam <at> alphapapa.net>, Eli Zaretskii <eliz <at> gnu.org>,
 phillip.lord <at> russet.org.uk, 63513-done <at> debbugs.gnu.org,
 Joseph Turner <joseph <at> breatheoutbreathe.in>
Subject: Re: bug#63513: [PATCH] Make persist-defvar work with records and
 hash tables
Date: Fri, 08 Sep 2023 17:06:10 +0000

Stefan Monnier <monnier <at> iro.umontreal.ca> writes:

>>>> Here you go!
>>> Thanks, pushed!
>> May it be possible to promote `persist-hash-equal' and
>> `persist-copy-tree' to common subr.el functions?
>> The topic of comparing hash tables has been previously discussed in
>> https://yhetil.org/emacs-devel/871qvz4kdw.fsf <at> localhost/
>
> Hmm... AFAICT `persist-copy-tree` is a copy of our current `copy-tree`,
> so there's nothing to do there :-)

Err...
Then, wouldn't it be better to contribute this function to compat.el and
use it from there?

> As for `persist-hash-equal` (which is actually called `persist-equal`,
> AFAICT), I think if we want to move it out of `persist.el` we need to
> improve it so it also looks inside hash-tables that occur within
> vectors, conses, etc...
>
> Maybe, like Sam Steingold suggested, we should simply consider the
> current treatment of `equal` on hash-tables to be a bug.

That thread had no progress for quite a while.
I thought that having `hash-equal' function could at least be an
improvement (clearly, it is being used 🙂).

But if you have an idea how to move the original discussion forward,
please share it. AFAIU, no actionable conclusion have been reached in
the linked thread.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>

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

From: Joseph Turner <joseph <at> breatheoutbreathe.in>
To: Ihor Radchenko <yantar92 <at> posteo.net>
Cc: Adam Porter <adam <at> alphapapa.net>, Eli Zaretskii <eliz <at> gnu.org>,
 63513-done <at> debbugs.gnu.org, Stefan Monnier <monnier <at> iro.umontreal.ca>,
 phillip.lord <at> russet.org.uk
Subject: Re: bug#63513: [PATCH] Make persist-defvar work with records and
 hash tables
Date: Fri, 08 Sep 2023 10:10:58 -0700

Ihor Radchenko <yantar92 <at> posteo.net> writes:

> Stefan Monnier <monnier <at> iro.umontreal.ca> writes:
>
>>>>> Here you go!
>>>> Thanks, pushed!
>>> May it be possible to promote `persist-hash-equal' and
>>> `persist-copy-tree' to common subr.el functions?
>>> The topic of comparing hash tables has been previously discussed in
>>> https://yhetil.org/emacs-devel/871qvz4kdw.fsf <at> localhost/
>>
>> Hmm... AFAICT `persist-copy-tree` is a copy of our current `copy-tree`,
>> so there's nothing to do there :-)
>
> Err...
> Then, wouldn't it be better to contribute this function to compat.el and
> use it from there?

The new behavior of copy-tree has already been added to compat.el:

https://github.com/emacs-compat/compat/pull/25

However, that change currently only exists in compat's emacs-30 branch.

I did not know if it was acceptable for persist.el to require compat
when I wrote these patches. If we agree that it is acceptable, then I'm
happy to submit a patch to replace (persist-copy-tree ...) with
(compat-call copy-tree ...), but we'll have to wait to apply the patch
until after the compat.el emacs-30 branch is merged into master.

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

From: Ihor Radchenko <yantar92 <at> posteo.net>
To: Joseph Turner <joseph <at> breatheoutbreathe.in>
Cc: 63513-done <at> debbugs.gnu.org, Daniel Mendler <mail <at> daniel-mendler.de>,
 Stefan Monnier <monnier <at> iro.umontreal.ca>, Adam Porter <adam <at> alphapapa.net>,
 Eli Zaretskii <eliz <at> gnu.org>, phillip.lord <at> russet.org.uk
Subject: Re: bug#63513: [PATCH] Make persist-defvar work with records and
 hash tables
Date: Sat, 09 Sep 2023 10:01:34 +0000

Joseph Turner <joseph <at> breatheoutbreathe.in> writes:

>> Then, wouldn't it be better to contribute this function to compat.el and
>> use it from there?
>
> The new behavior of copy-tree has already been added to compat.el:
>
> https://github.com/emacs-compat/compat/pull/25
>
> However, that change currently only exists in compat's emacs-30 branch.

I see.

> I did not know if it was acceptable for persist.el to require compat
> when I wrote these patches. If we agree that it is acceptable, then I'm
> happy to submit a patch to replace (persist-copy-tree ...) with
> (compat-call copy-tree ...), but we'll have to wait to apply the patch
> until after the compat.el emacs-30 branch is merged into master.

AFAIU, the recommended way to implement compat function definitions that
are not yet added to compat.el is using `compat-defun' + `compat-call'.
Then, one can simply drop `compat-defun' after the function is finally
release with compat.el without touching the rest of the code.

CCing Daniel.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>

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

From: Daniel Mendler <mail <at> daniel-mendler.de>
To: Ihor Radchenko <yantar92 <at> posteo.net>,
 Joseph Turner <joseph <at> breatheoutbreathe.in>
Cc: Adam Porter <adam <at> alphapapa.net>, Eli Zaretskii <eliz <at> gnu.org>,
 63513-done <at> debbugs.gnu.org, Stefan Monnier <monnier <at> iro.umontreal.ca>,
 phillip.lord <at> russet.org.uk
Subject: Re: bug#63513: [PATCH] Make persist-defvar work with records and hash
 tables
Date: Sat, 9 Sep 2023 12:15:31 +0200

On 9/9/23 12:01, Ihor Radchenko wrote:
> Joseph Turner <joseph <at> breatheoutbreathe.in> writes:
> 
>>> Then, wouldn't it be better to contribute this function to compat.el and
>>> use it from there?
>>
>> The new behavior of copy-tree has already been added to compat.el:
>>
>> https://github.com/emacs-compat/compat/pull/25
>>
>> However, that change currently only exists in compat's emacs-30 branch.
> 
> I see.
>
>> I did not know if it was acceptable for persist.el to require compat
>> when I wrote these patches. If we agree that it is acceptable, then I'm
>> happy to submit a patch to replace (persist-copy-tree ...) with
>> (compat-call copy-tree ...), but we'll have to wait to apply the patch
>> until after the compat.el emacs-30 branch is merged into master.
> 
> AFAIU, the recommended way to implement compat function definitions that
> are not yet added to compat.el is using `compat-defun' + `compat-call'.
> Then, one can simply drop `compat-defun' after the function is finally
> release with compat.el without touching the rest of the code.

`compat-call` is meant to be used to call "extended functions", for
example functions with additional arguments. See the Compat manual for
details.

The macros from compat-macs.el (`compat-defun` etc.) are internal as
documented in the file compat-macs.el. These macros must not be used
outside Compat.

So using Compat here has to wait until compat-30.x is released.

Daniel

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

From: Ihor Radchenko <yantar92 <at> posteo.net>
To: Daniel Mendler <mail <at> daniel-mendler.de>
Cc: 63513-done <at> debbugs.gnu.org, Joseph Turner <joseph <at> breatheoutbreathe.in>,
 Stefan Monnier <monnier <at> iro.umontreal.ca>, Adam Porter <adam <at> alphapapa.net>,
 Eli Zaretskii <eliz <at> gnu.org>, phillip.lord <at> russet.org.uk
Subject: Re: bug#63513: [PATCH] Make persist-defvar work with records and
 hash tables
Date: Sat, 09 Sep 2023 11:35:02 +0000

Daniel Mendler <mail <at> daniel-mendler.de> writes:

>> AFAIU, the recommended way to implement compat function definitions that
>> are not yet added to compat.el is using `compat-defun' + `compat-call'.
>> Then, one can simply drop `compat-defun' after the function is finally
>> release with compat.el without touching the rest of the code.
>
> `compat-call` is meant to be used to call "extended functions", for
> example functions with additional arguments. See the Compat manual for
> details.
>
> The macros from compat-macs.el (`compat-defun` etc.) are internal as
> documented in the file compat-macs.el. These macros must not be used
> outside Compat.
>
> So using Compat here has to wait until compat-30.x is released.

And do I understand correctly that compat-30 will only be released after
Emacs 30 is released? If so, it is awkward for :core packages.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>

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

From: Daniel Mendler <mail <at> daniel-mendler.de>
To: Ihor Radchenko <yantar92 <at> posteo.net>
Cc: 63513-done <at> debbugs.gnu.org, Joseph Turner <joseph <at> breatheoutbreathe.in>,
 Stefan Monnier <monnier <at> iro.umontreal.ca>, Adam Porter <adam <at> alphapapa.net>,
 Eli Zaretskii <eliz <at> gnu.org>, phillip.lord <at> russet.org.uk
Subject: Re: bug#63513: [PATCH] Make persist-defvar work with records and hash
 tables
Date: Sat, 9 Sep 2023 13:57:27 +0200

On 9/9/23 13:35, Ihor Radchenko wrote:
>> So using Compat here has to wait until compat-30.x is released.
> 
> And do I understand correctly that compat-30 will only be released after
> Emacs 30 is released? If so, it is awkward for :core packages.

compat-30 can be released as soon as Emacs 30 has been reasonably
stabilized, e.g., when the emacs-30 branch has been frozen, or a bit
before that. We cannot release much earlier since APIs may still change
and it is probably undesired to release unfinished APIs to the public
too early. For reference, I've created the compat-29.1.1 release around
the day that Eli announced the emacs-29 branch freeze.

Daniel

Previous Next

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