GNU bug report logs - #76123
[PATCH] Eliminate savehist duplicated symbols

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 76123 in the body.
You can then email your comments to 76123 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: Ship Mints <shipmints <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: [PATCH] Eliminate savehist duplicated symbols
Date: Fri, 7 Feb 2025 15:05:40 -0500

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

This patch avoids savehist saving symbols duplicated between
'savehist-minibuffer-history-variables' and
'savehist-additional-variables'. I've accumulated symbols on
'savehist-additional-variables' that wind up, over time, in the master
minibuffer list documented as being dynamic.

  The contents of this variable is built while Emacs is running,
  and saved along with minibuffer history.

savehist had no defensive mechanism until now. The patch should do no harm
to users and will transparently save some resources.

TIA,

-Stephane

[Message part 2 (text/html, inline)]

[0001-Eliminate-savehist-duplicated-symbols.patch (application/octet-stream, attachment)]

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

From: Stefan Kangas <stefankangas <at> gmail.com>
To: Ship Mints <shipmints <at> gmail.com>, 76123 <at> debbugs.gnu.org
Subject: Re: bug#76123: [PATCH] Eliminate savehist duplicated symbols
Date: Fri, 7 Feb 2025 12:45:12 -0800

Ship Mints <shipmints <at> gmail.com> writes:

> This patch avoids savehist saving symbols duplicated between
> 'savehist-minibuffer-history-variables' and
> 'savehist-additional-variables'. I've accumulated symbols on
> 'savehist-additional-variables' that wind up, over time, in the master
> minibuffer list documented as being dynamic.
>
>   The contents of this variable is built while Emacs is running,
>   and saved along with minibuffer history.
>
> savehist had no defensive mechanism until now. The patch should do no harm
> to users and will transparently save some resources.

Would it be worth adding a test for this?

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

From: Ship Mints <shipmints <at> gmail.com>
To: Stefan Kangas <stefankangas <at> gmail.com>
Cc: 76123 <at> debbugs.gnu.org
Subject: Re: bug#76123: [PATCH] Eliminate savehist duplicated symbols
Date: Fri, 7 Feb 2025 15:57:55 -0500

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

I kinda doubt it is worth it. There are currently no savehist tests at all
that I can see. But if you really, really want one, I'll start a test jig.

On Fri, Feb 7, 2025 at 3:45 PM Stefan Kangas <stefankangas <at> gmail.com> wrote:

> Ship Mints <shipmints <at> gmail.com> writes:
>
> > This patch avoids savehist saving symbols duplicated between
> > 'savehist-minibuffer-history-variables' and
> > 'savehist-additional-variables'. I've accumulated symbols on
> > 'savehist-additional-variables' that wind up, over time, in the master
> > minibuffer list documented as being dynamic.
> >
> >   The contents of this variable is built while Emacs is running,
> >   and saved along with minibuffer history.
> >
> > savehist had no defensive mechanism until now. The patch should do no
> harm
> > to users and will transparently save some resources.
>
> Would it be worth adding a test for this?
>

[Message part 2 (text/html, inline)]

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

From: Stefan Kangas <stefankangas <at> gmail.com>
To: Ship Mints <shipmints <at> gmail.com>
Cc: 76123 <at> debbugs.gnu.org
Subject: Re: bug#76123: [PATCH] Eliminate savehist duplicated symbols
Date: Fri, 7 Feb 2025 13:18:52 -0800

Ship Mints <shipmints <at> gmail.com> writes:

> I kinda doubt it is worth it. There are currently no savehist tests at all
> that I can see. But if you really, really want one, I'll start a test jig.

No, I won't insist on it, especially since there are no tests.

That said, adding even just a few basic tests for savehist would be
nice.  Maybe not in this bug report, though.

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

From: Ship Mints <shipmints <at> gmail.com>
To: Stefan Kangas <stefankangas <at> gmail.com>
Cc: 76123 <at> debbugs.gnu.org
Subject: Re: bug#76123: [PATCH] Eliminate savehist duplicated symbols
Date: Fri, 7 Feb 2025 16:20:18 -0500

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

Let's handle this tiny bug fix separately.

I'll take a look and create a simple test to get it started another bug
report.

On Fri, Feb 7, 2025 at 4:18 PM Stefan Kangas <stefankangas <at> gmail.com> wrote:

> Ship Mints <shipmints <at> gmail.com> writes:
>
> > I kinda doubt it is worth it. There are currently no savehist tests at
> all
> > that I can see. But if you really, really want one, I'll start a test
> jig.
>
> No, I won't insist on it, especially since there are no tests.
>
> That said, adding even just a few basic tests for savehist would be
> nice.  Maybe not in this bug report, though.
>

[Message part 2 (text/html, inline)]

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

From: Stefan Kangas <stefankangas <at> gmail.com>
To: Ship Mints <shipmints <at> gmail.com>
Cc: 76123 <at> debbugs.gnu.org
Subject: Re: bug#76123: [PATCH] Eliminate savehist duplicated symbols
Date: Fri, 7 Feb 2025 13:59:56 -0800

Ship Mints <shipmints <at> gmail.com> writes:

> Let's handle this tiny bug fix separately.
>
> I'll take a look and create a simple test to get it started another bug
> report.

SGTM.  Thanks for doing that.

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

From: Ship Mints <shipmints <at> gmail.com>
To: Stefan Kangas <stefankangas <at> gmail.com>
Cc: 76123 <at> debbugs.gnu.org
Subject: Re: bug#76123: [PATCH] Eliminate savehist duplicated symbols
Date: Fri, 7 Feb 2025 19:04:27 -0500

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

Stefan,

Revised patch attached with two tests to get savehist tests started. One is
a basic functions test and the other a duplicate test. They probably could
be combined into one test, tbh, but not precisely sure what the Emacs Core
testing regime is for functionality.

I tested on both Emacs 29 with the duplicate issue and with the duplicate
avoidance patch applied.

-Stephane

On Fri, Feb 7, 2025 at 4:59 PM Stefan Kangas <stefankangas <at> gmail.com> wrote:

> Ship Mints <shipmints <at> gmail.com> writes:
>
> > Let's handle this tiny bug fix separately.
> >
> > I'll take a look and create a simple test to get it started another bug
> > report.
>
> SGTM.  Thanks for doing that.
>

[Message part 2 (text/html, inline)]

[0001-Eliminate-savehist-duplicated-symbols.patch (application/octet-stream, attachment)]

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Ship Mints <shipmints <at> gmail.com>
Cc: 76123 <at> debbugs.gnu.org, stefankangas <at> gmail.com
Subject: Re: bug#76123: [PATCH] Eliminate savehist duplicated symbols
Date: Sat, 08 Feb 2025 10:34:52 +0200

> Cc: 76123 <at> debbugs.gnu.org
> From: Ship Mints <shipmints <at> gmail.com>
> Date: Fri, 7 Feb 2025 19:04:27 -0500
> 
> Revised patch attached with two tests to get savehist tests started. One is a basic functions test and the
> other a duplicate test. They probably could be combined into one test, tbh, but not precisely sure what the
> Emacs Core testing regime is for functionality.
> 
> I tested on both Emacs 29 with the duplicate issue and with the duplicate avoidance patch applied.

Thanks.  There's one more nit: I think this change needs to be called
out in NEWS.  It changes user-facing behavior, so users should be
informed about that.

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

From: Ship Mints <shipmints <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 76123 <at> debbugs.gnu.org, stefankangas <at> gmail.com
Subject: Re: bug#76123: [PATCH] Eliminate savehist duplicated symbols
Date: Sat, 8 Feb 2025 05:38:47 -0500

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

Surely. Revised patch attached with a NEWS entry.

I did not yet combine the two tests (the code is nearly duplicated) if
that's what you think should be done so if you want me to do that, I can
today.

On Sat, Feb 8, 2025 at 3:35 AM Eli Zaretskii <eliz <at> gnu.org> wrote:

> > Cc: 76123 <at> debbugs.gnu.org
> > From: Ship Mints <shipmints <at> gmail.com>
> > Date: Fri, 7 Feb 2025 19:04:27 -0500
> >
> > Revised patch attached with two tests to get savehist tests started. One
> is a basic functions test and the
> > other a duplicate test. They probably could be combined into one test,
> tbh, but not precisely sure what the
> > Emacs Core testing regime is for functionality.
> >
> > I tested on both Emacs 29 with the duplicate issue and with the
> duplicate avoidance patch applied.
>
> Thanks.  There's one more nit: I think this change needs to be called
> out in NEWS.  It changes user-facing behavior, so users should be
> informed about that.
>

[Message part 2 (text/html, inline)]

[0001-Eliminate-savehist-duplicated-symbols.patch (application/octet-stream, attachment)]

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Ship Mints <shipmints <at> gmail.com>
Cc: 76123-done <at> debbugs.gnu.org, stefankangas <at> gmail.com
Subject: Re: bug#76123: [PATCH] Eliminate savehist duplicated symbols
Date: Sat, 08 Feb 2025 14:07:58 +0200

> From: Ship Mints <shipmints <at> gmail.com>
> Date: Sat, 8 Feb 2025 05:38:47 -0500
> Cc: stefankangas <at> gmail.com, 76123 <at> debbugs.gnu.org
> 
> Surely. Revised patch attached with a NEWS entry.

Thanks, installed on master, and closing the bug.

> I did not yet combine the two tests (the code is nearly duplicated) if that's what you think should be done so
> if you want me to do that, I can today.

I see no need.

Previous Next

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