GNU bug report logs - #53638
[PATCH] newsticker better path handling and data storage

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 53638 in the body.
You can then email your comments to 53638 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: mailj2 <at> protonmail.com
To: bug-gnu-emacs <at> gnu.org
Subject: [PATCH] newsticker better path handling and data storage
Date: Sun, 30 Jan 2022 19:47:03 +0000

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

The way newsticker currently handles paths involves directly concating
"/", which only works for UNIX systems.

Additionally, since newsticker uses `prin1-to-string' for storing
groups and cache, it should be enforced that `print-level' and
`print-length' are set to nil for the operation just as a precaution (it
happened to me that I manually set these once, which resulted in the
newsticker cache being corrupted).

This patch should fix both of these issues.

[newsticker-patch.diff (text/x-patch, attachment)]

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

From: Robert Pluim <rpluim <at> gmail.com>
To: 53638 <at> debbugs.gnu.org
Cc: mailj2 <at> protonmail.com
Subject: Re: bug#53638: [PATCH] newsticker better path handling and data
 storage
Date: Mon, 31 Jan 2022 11:23:47 +0100

>>>>> On Sun, 30 Jan 2022 19:47:03 +0000, mailj2--- via "Bug reports for GNU Emacs, the Swiss army knife of text editors" <bug-gnu-emacs <at> gnu.org> said:

    mailj2> The way newsticker currently handles paths involves directly concating
    mailj2> "/", which only works for UNIX systems.

emacs-28 has `file-name-concat' for this very purpose, perhaps you
could respin to use that instead.

    mailj2> Additionally, since newsticker uses `prin1-to-string' for storing
    mailj2> groups and cache, it should be enforced that `print-level' and
    mailj2> `print-length' are set to nil for the operation just as a precaution (it
    mailj2> happened to me that I manually set these once, which resulted in the
    mailj2> newsticker cache being corrupted).

OK. It would be good to have a ChangeLog style commit message as well,
see CONTRIBUTE for details. 'C-x 4 a' can help with writing it.

Robert
-- 

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: mailj2 <at> protonmail.com
Cc: 53638 <at> debbugs.gnu.org
Subject: Re: bug#53638: [PATCH] newsticker better path handling and data
 storage
Date: Mon, 31 Jan 2022 14:19:21 +0200

> Date: Sun, 30 Jan 2022 19:47:03 +0000
> From: mailj2--- via "Bug reports for GNU Emacs,
>  the Swiss army knife of text editors" <bug-gnu-emacs <at> gnu.org>
> 
> The way newsticker currently handles paths involves directly concating
> "/", which only works for UNIX systems.

The canonical way of doing that is to use expand-file-name.

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

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: mailj2 <at> protonmail.com
Cc: 53638 <at> debbugs.gnu.org
Subject: Re: bug#53638: [PATCH] newsticker better path handling and data
 storage
Date: Mon, 31 Jan 2022 16:47:13 +0100

mailj2 <at> protonmail.com writes:

> The way newsticker currently handles paths involves directly concating
> "/", which only works for UNIX systems.

Can you rework the patch to use `expand-file-name', which is how
file names are supposed to be constructed?

> Additionally, since newsticker uses `prin1-to-string' for storing
> groups and cache, it should be enforced that `print-level' and
> `print-length' are set to nil for the operation just as a precaution (it
> happened to me that I manually set these once, which resulted in the
> newsticker cache being corrupted).

That part of the patch looks good.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

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

From: mailj2 <at> protonmail.com
To: 53638 <at> debbugs.gnu.org
Cc: Lars Ingebrigtsen <larsi <at> gnus.org>
Subject: Re: bug#53638: [PATCH] newsticker better path handling and data
 storage
Date: Mon, 31 Jan 2022 20:09:58 +0000

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

"Lars Ingebrigtsen" <larsi <at> gnus.org> writes:

> Can you rework the patch to use
> `expand-file-name', which is how file names are supposed to be constructed?

I've refactored it to use `expand-file-name' and `file-name-concat'. It
appears the current convention (correct me if I'm wrong) is to use
`expand-file-name' for joining only one path and one directory, and
`file-name-concat' for multiple together.

[newsticker-patch-2.diff (text/x-patch, attachment)]

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: mailj2 <at> protonmail.com
Cc: 53638 <at> debbugs.gnu.org, larsi <at> gnus.org
Subject: Re: bug#53638: [PATCH] newsticker better path handling and data
 storage
Date: Tue, 01 Feb 2022 05:25:56 +0200

> Cc: Lars Ingebrigtsen <larsi <at> gnus.org>
> Date: Mon, 31 Jan 2022 20:09:58 +0000
> From: mailj2--- via "Bug reports for GNU Emacs,
>  the Swiss army knife of text editors" <bug-gnu-emacs <at> gnu.org>
> 
> "Lars Ingebrigtsen" <larsi <at> gnus.org> writes:
> 
> > Can you rework the patch to use
> > `expand-file-name', which is how file names are supposed to be constructed?
> 
> I've refactored it to use `expand-file-name' and `file-name-concat'. It
> appears the current convention (correct me if I'm wrong) is to use
> `expand-file-name' for joining only one path and one directory, and
> `file-name-concat' for multiple together.

No, you can chain calls to expand-file-name as you need:

  (expand-file-name "foo" (expand-file-name "bar" my-directory))

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

From: Xavier Capaldi <xcapaldi <at> scribo.biz>
To: 53638 <at> debbugs.gnu.org
Subject: Re: bug#53638: [PATCH] newsticker better path handling and data
Date: Sun, 15 May 2022 23:50:22 -0400

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

Not sure if this bug report/patch is blocked but I faced the
`prin1-to-string` issue which is fixed by the patch here. I've attached
an updated patch that uses chained calls to `expand-file-name` instead
of `file-name-concat` since that was the last issue mentioned with the
patch.

[newsticker-patch-3.diff (text/x-patch, attachment)]

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

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Xavier Capaldi <xcapaldi <at> scribo.biz>
Cc: 53638 <at> debbugs.gnu.org
Subject: Re: bug#53638: [PATCH] newsticker better path handling and data
Date: Mon, 16 May 2022 14:04:52 +0200

Xavier Capaldi <xcapaldi <at> scribo.biz> writes:

> Not sure if this bug report/patch is blocked but I faced the
> `prin1-to-string` issue which is fixed by the patch here. I've attached
> an updated patch that uses chained calls to `expand-file-name` instead
> of `file-name-concat` since that was the last issue mentioned with the
> patch.

Thanks.  I've now applied the patch (with a number of changes) and
pushed to Emacs 29.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Previous Next

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