GNU bug report logs - #50965
Allow nil third argument for mapconcat

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 50965 in the body.
You can then email your comments to 50965 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: Stefan Kangas <stefan <at> marxist.se>
To: bug-gnu-emacs <at> gnu.org
Subject: Allow nil third argument for mapconcat
Date: Sat, 2 Oct 2021 15:22:44 +0200

Severity: wishlist

Please consider allowing an empty third argument for mapconcat that
means the empty string, i.e.:

    (mapconcat FUNCTION SEQUENCE SEPARATOR)

Becomes:

    (mapconcat FUNCTION SEQUENCE &optional SEPARATOR)

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

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Stefan Kangas <stefan <at> marxist.se>
Cc: 50965 <at> debbugs.gnu.org
Subject: Re: bug#50965: Allow nil third argument for mapconcat
Date: Sat, 02 Oct 2021 15:50:44 +0200

Stefan Kangas <stefan <at> marxist.se> writes:

> Please consider allowing an empty third argument for mapconcat that
> means the empty string, i.e.:
>
>     (mapconcat FUNCTION SEQUENCE SEPARATOR)
>
> Becomes:
>
>     (mapconcat FUNCTION SEQUENCE &optional SEPARATOR)

I think that makes sense -- SEPARATOR is optional in the related

(string-join strings &optional separator)

function.

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

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

From: Stefan Kangas <stefan <at> marxist.se>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: 50965 <at> debbugs.gnu.org
Subject: Re: bug#50965: Allow nil third argument for mapconcat
Date: Sat, 2 Oct 2021 22:12:32 +0200

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

> I think that makes sense -- SEPARATOR is optional in the related
>
> (string-join strings &optional separator)
>
> function.

Aha, so an explicit nil third argument is already treated as the empty string:

    (mapconcat #'identity '("foo" "bar") nil)

This is the case also in 27.2.

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

From: Stefan Kangas <stefan <at> marxist.se>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: 50965 <at> debbugs.gnu.org
Subject: Re: bug#50965: Allow nil third argument for mapconcat
Date: Sun, 3 Oct 2021 01:03:54 +0200

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

tags 50965 + patch
thanks

Stefan Kangas <stefan <at> marxist.se> writes:

> Aha, so an explicit nil third argument is already treated as the empty string:
>
>     (mapconcat #'identity '("foo" "bar") nil)

How does the attached patch look?  I'm assuming this should go to
master at this point.

[0001-Make-third-mapconcat-argument-optional.patch (application/octet-stream, attachment)]

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Stefan Kangas <stefan <at> marxist.se>
Cc: larsi <at> gnus.org, 50965 <at> debbugs.gnu.org
Subject: Re: bug#50965: Allow nil third argument for mapconcat
Date: Sun, 03 Oct 2021 08:56:23 +0300

> From: Stefan Kangas <stefan <at> marxist.se>
> Date: Sun, 3 Oct 2021 01:03:54 +0200
> Cc: 50965 <at> debbugs.gnu.org
> 
> +Optional argument SEPARATOR must be a string, a vector, or a list of
> +characters; nil is the empty string."

nil is not a string, so saying that is not the best wording.  I
suggest "nil stands for an empty string." instead.

> +Optional argument SEPARATOR must be a string, a vector, or a list of
> +characters; nil is the empty string.

Likewise.

> +(ert-deftest fns-tests-mapconcat ()
> +  (should (string= (mapconcat #'identity '() "_") ""))
> +  (should (string= (mapconcat #'identity '("A") "_") "A"))
> +  (should (string= (mapconcat #'identity '("A" "B") "_") "A_B"))
> +  (should (string= (mapconcat #'identity '("A" "B" "C") "_") "A_B_C"))
> +  (should (string= (mapconcat #'identity '("a" "b")) "ab")))

Please be sure to test with vectors, bool-vectors, list of characters,
and non-ASCII strings as well.

> I'm assuming this should go to master at this point.

Yes, thanks.

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

From: Stefan Kangas <stefan <at> marxist.se>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: larsi <at> gnus.org, 50965 <at> debbugs.gnu.org
Subject: Re: bug#50965: Allow nil third argument for mapconcat
Date: Sun, 3 Oct 2021 18:34:31 +0000

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

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

>> +Optional argument SEPARATOR must be a string, a vector, or a list of
>> +characters; nil is the empty string."
>
> nil is not a string, so saying that is not the best wording.  I
> suggest "nil stands for an empty string." instead.

Yes, that's better.  Fixed.

>> +(ert-deftest fns-tests-mapconcat ()
>> +  (should (string= (mapconcat #'identity '() "_") ""))
>> +  (should (string= (mapconcat #'identity '("A") "_") "A"))
>> +  (should (string= (mapconcat #'identity '("A" "B") "_") "A_B"))
>> +  (should (string= (mapconcat #'identity '("A" "B" "C") "_") "A_B_C"))
>> +  (should (string= (mapconcat #'identity '("a" "b")) "ab")))
>
> Please be sure to test with vectors, bool-vectors, list of characters,
> and non-ASCII strings as well.

I've added such tests.  Please check especially the bool-vector case,
as I was unsure about what exactly to test for.  I put this:

    (should (string= (mapconcat #'identity [nil nil] "") ""))
    (should-error (mapconcat #'identity [nil nil t])
                  :type 'wrong-type-argument)

>> I'm assuming this should go to master at this point.
>
> Yes, thanks.

New patch attached.

[0001-Make-mapconcat-argument-separator-optional.patch (text/x-diff, attachment)]

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Stefan Kangas <stefan <at> marxist.se>
Cc: larsi <at> gnus.org, 50965 <at> debbugs.gnu.org
Subject: Re: bug#50965: Allow nil third argument for mapconcat
Date: Tue, 05 Oct 2021 16:02:01 +0300

> From: Stefan Kangas <stefan <at> marxist.se>
> Date: Sun, 3 Oct 2021 18:34:31 +0000
> Cc: larsi <at> gnus.org, 50965 <at> debbugs.gnu.org
> 
> New patch attached.

LGTM, thanks.

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

From: Stefan Kangas <stefan <at> marxist.se>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: larsi <at> gnus.org, 50965 <at> debbugs.gnu.org
Subject: Re: bug#50965: Allow nil third argument for mapconcat
Date: Tue, 5 Oct 2021 09:39:36 -0400

tags 50965 fixed
close 50965 29.1
thanks

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

> LGTM, thanks.

Thanks, pushed to master as commit d652efcd08.

Previous Next

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