* bug#50965: Allow nil third argument for mapconcat
@ 2021-10-02 13:22 Stefan Kangas
2021-10-02 13:50 ` Lars Ingebrigtsen
0 siblings, 1 reply; 8+ messages in thread
From: Stefan Kangas @ 2021-10-02 13:22 UTC (permalink / raw)
To: 50965
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)
^ permalink raw reply [flat|nested] 8+ messages in thread* bug#50965: Allow nil third argument for mapconcat 2021-10-02 13:22 bug#50965: Allow nil third argument for mapconcat Stefan Kangas @ 2021-10-02 13:50 ` Lars Ingebrigtsen 2021-10-02 20:12 ` Stefan Kangas 0 siblings, 1 reply; 8+ messages in thread From: Lars Ingebrigtsen @ 2021-10-02 13:50 UTC (permalink / raw) To: Stefan Kangas; +Cc: 50965 Stefan Kangas <stefan@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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* bug#50965: Allow nil third argument for mapconcat 2021-10-02 13:50 ` Lars Ingebrigtsen @ 2021-10-02 20:12 ` Stefan Kangas 2021-10-02 23:03 ` Stefan Kangas 0 siblings, 1 reply; 8+ messages in thread From: Stefan Kangas @ 2021-10-02 20:12 UTC (permalink / raw) To: Lars Ingebrigtsen; +Cc: 50965 Lars Ingebrigtsen <larsi@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. ^ permalink raw reply [flat|nested] 8+ messages in thread
* bug#50965: Allow nil third argument for mapconcat 2021-10-02 20:12 ` Stefan Kangas @ 2021-10-02 23:03 ` Stefan Kangas 2021-10-03 5:56 ` Eli Zaretskii 0 siblings, 1 reply; 8+ messages in thread From: Stefan Kangas @ 2021-10-02 23:03 UTC (permalink / raw) To: Lars Ingebrigtsen; +Cc: 50965 [-- Attachment #1: Type: text/plain, Size: 290 bytes --] tags 50965 + patch thanks Stefan Kangas <stefan@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. [-- Attachment #2: 0001-Make-third-mapconcat-argument-optional.patch --] [-- Type: application/octet-stream, Size: 3358 bytes --] From ac2f687c45c6a298fbc1cbfd6b7b21f5f17c3d3c Mon Sep 17 00:00:00 2001 From: Stefan Kangas <stefan@marxist.se> Date: Sun, 3 Oct 2021 00:41:13 +0200 Subject: [PATCH] Make third mapconcat argument optional * src/fns.c (Fmapconcat): Make 'separator' argument optional. (Bug#50965) * test/src/fns-tests.el (fns-tests-mapconcat): New test. * lisp/emacs-lisp/subr-x.el (string-join): Document 'separator' argument better. --- etc/NEWS | 3 +++ lisp/emacs-lisp/subr-x.el | 4 +++- src/fns.c | 8 ++++++-- test/src/fns-tests.el | 7 +++++++ 4 files changed, 19 insertions(+), 3 deletions(-) diff --git a/etc/NEWS b/etc/NEWS index 8c22230daf..7be043fc99 100644 --- a/etc/NEWS +++ b/etc/NEWS @@ -80,6 +80,9 @@ Emacs buffers, like indentation and the like. The new ert function \f * Lisp Changes in Emacs 29.1 +** Third 'mapconcat' argument 'separator' is now optional. +An explicit nil always meant the empty string, now it can be left out. + \f * Changes in Emacs 29.1 on Non-Free Operating Systems diff --git a/lisp/emacs-lisp/subr-x.el b/lisp/emacs-lisp/subr-x.el index 91ebbf9fb9..2bdbdaab74 100644 --- a/lisp/emacs-lisp/subr-x.el +++ b/lisp/emacs-lisp/subr-x.el @@ -208,7 +208,9 @@ string-empty-p (string= string "")) (defsubst string-join (strings &optional separator) - "Join all STRINGS using SEPARATOR." + "Join all STRINGS using SEPARATOR. +Optional argument SEPARATOR must be a string, a vector, or a list of +characters; nil is the empty string." (mapconcat #'identity strings separator)) (define-obsolete-function-alias 'string-reverse 'reverse "25.1") diff --git a/src/fns.c b/src/fns.c index a72e41aee5..f77b6009bd 100644 --- a/src/fns.c +++ b/src/fns.c @@ -2852,12 +2852,16 @@ mapcar1 (EMACS_INT leni, Lisp_Object *vals, Lisp_Object fn, Lisp_Object seq) return leni; } -DEFUN ("mapconcat", Fmapconcat, Smapconcat, 3, 3, 0, +DEFUN ("mapconcat", Fmapconcat, Smapconcat, 2, 3, 0, doc: /* Apply FUNCTION to each element of SEQUENCE, and concat the results as strings. In between each pair of results, stick in SEPARATOR. Thus, " " as SEPARATOR results in spaces between the values returned by FUNCTION. + SEQUENCE may be a list, a vector, a bool-vector, or a string. -SEPARATOR must be a string, a vector, or a list of characters. + +Optional argument SEPARATOR must be a string, a vector, or a list of +characters; nil is the empty string. + FUNCTION must be a function of one argument, and must return a value that is a sequence of characters: either a string, or a vector or list of numbers that are valid character codepoints. */) diff --git a/test/src/fns-tests.el b/test/src/fns-tests.el index 5759457209..a771790b68 100644 --- a/test/src/fns-tests.el +++ b/test/src/fns-tests.el @@ -430,6 +430,13 @@ fns-tests-hash-buffer (buffer-hash)) (sha1 "foo")))) +(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"))) + (ert-deftest fns-tests-mapcan () (should-error (mapcan)) (should-error (mapcan #'identity)) -- 2.33.0 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* bug#50965: Allow nil third argument for mapconcat 2021-10-02 23:03 ` Stefan Kangas @ 2021-10-03 5:56 ` Eli Zaretskii 2021-10-03 18:34 ` Stefan Kangas 0 siblings, 1 reply; 8+ messages in thread From: Eli Zaretskii @ 2021-10-03 5:56 UTC (permalink / raw) To: Stefan Kangas; +Cc: larsi, 50965 > From: Stefan Kangas <stefan@marxist.se> > Date: Sun, 3 Oct 2021 01:03:54 +0200 > Cc: 50965@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. ^ permalink raw reply [flat|nested] 8+ messages in thread
* bug#50965: Allow nil third argument for mapconcat 2021-10-03 5:56 ` Eli Zaretskii @ 2021-10-03 18:34 ` Stefan Kangas 2021-10-05 13:02 ` Eli Zaretskii 0 siblings, 1 reply; 8+ messages in thread From: Stefan Kangas @ 2021-10-03 18:34 UTC (permalink / raw) To: Eli Zaretskii; +Cc: larsi, 50965 [-- Attachment #1: Type: text/plain, Size: 1172 bytes --] Eli Zaretskii <eliz@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. [-- Attachment #2: 0001-Make-mapconcat-argument-separator-optional.patch --] [-- Type: text/x-diff, Size: 5918 bytes --] From 7646c687a899f8bba7e71c6f5f59f4262bb7d592 Mon Sep 17 00:00:00 2001 From: Stefan Kangas <stefan@marxist.se> Date: Sun, 3 Oct 2021 19:24:31 +0200 Subject: [PATCH] Make 'mapconcat' argument 'separator' optional * src/fns.c (Fmapconcat): Make third 'separator' argument optional. (Bug#50965) * doc/lispref/functions.texi (Mapping Functions): Update documentation for above change. * test/src/fns-tests.el (fns-tests-mapconcat): New test. * doc/misc/cl.texi (Obsolete Setf Customization): Don't use third mapconcat argument in example. * lisp/emacs-lisp/subr-x.el (string-join): Doc fix. --- doc/lispref/functions.texi | 9 ++++----- doc/misc/cl.texi | 2 +- etc/NEWS | 4 ++++ lisp/emacs-lisp/subr-x.el | 4 +++- src/fns.c | 8 ++++++-- test/src/fns-tests.el | 17 +++++++++++++++++ 6 files changed, 35 insertions(+), 9 deletions(-) diff --git a/doc/lispref/functions.texi b/doc/lispref/functions.texi index c856557c3c..3163300184 100644 --- a/doc/lispref/functions.texi +++ b/doc/lispref/functions.texi @@ -961,14 +961,14 @@ Mapping Functions into a list. @code{mapc} always returns @var{sequence}. @end defun -@defun mapconcat function sequence separator +@defun mapconcat function sequence &optional separator @code{mapconcat} applies @var{function} to each element of @var{sequence}; the results, which must be sequences of characters (strings, vectors, or lists), are concatenated into a single string return value. Between each pair of result sequences, @code{mapconcat} inserts the characters from @var{separator}, which also must be a -string, or a vector or list of characters. @xref{Sequences Arrays -Vectors}. +string, or a vector or list of characters; a @code{nil} value is +treated as the empty string. @xref{Sequences Arrays Vectors}. The argument @var{function} must be a function that can take one argument and returns a sequence of characters: a string, a vector, or @@ -986,8 +986,7 @@ Mapping Functions @group (mapconcat (lambda (x) (format "%c" (1+ x))) - "HAL-8000" - "") + "HAL-8000") @result{} "IBM.9111" @end group @end example diff --git a/doc/misc/cl.texi b/doc/misc/cl.texi index a6c3c32c0e..f31dedae88 100644 --- a/doc/misc/cl.texi +++ b/doc/misc/cl.texi @@ -5024,7 +5024,7 @@ Obsolete Setf Customization @ignore (defmacro concatf (place &rest args) (gv-letplace (getter setter) place - (macroexp-let2 nil v (mapconcat 'identity args "") + (macroexp-let2 nil v (mapconcat 'identity args) (funcall setter `(concat ,getter ,v))))) @end ignore @end defmac diff --git a/etc/NEWS b/etc/NEWS index 20ed20308e..afd335163b 100644 --- a/etc/NEWS +++ b/etc/NEWS @@ -80,6 +80,10 @@ Emacs buffers, like indentation and the like. The new ert function \f * Lisp Changes in Emacs 29.1 ++++ +** Third 'mapconcat' argument 'separator' is now optional. +An explicit nil always meant the empty string, now it can be left out. + --- ** Themes can now be made obsolete. Using 'make-obsolete' on a theme is now supported. This will make diff --git a/lisp/emacs-lisp/subr-x.el b/lisp/emacs-lisp/subr-x.el index 91ebbf9fb9..7d1b6d85ed 100644 --- a/lisp/emacs-lisp/subr-x.el +++ b/lisp/emacs-lisp/subr-x.el @@ -208,7 +208,9 @@ string-empty-p (string= string "")) (defsubst string-join (strings &optional separator) - "Join all STRINGS using SEPARATOR." + "Join all STRINGS using SEPARATOR. +Optional argument SEPARATOR must be a string, a vector, or a list of +characters; nil stands for the empty string." (mapconcat #'identity strings separator)) (define-obsolete-function-alias 'string-reverse 'reverse "25.1") diff --git a/src/fns.c b/src/fns.c index a72e41aee5..61182852ba 100644 --- a/src/fns.c +++ b/src/fns.c @@ -2852,12 +2852,16 @@ mapcar1 (EMACS_INT leni, Lisp_Object *vals, Lisp_Object fn, Lisp_Object seq) return leni; } -DEFUN ("mapconcat", Fmapconcat, Smapconcat, 3, 3, 0, +DEFUN ("mapconcat", Fmapconcat, Smapconcat, 2, 3, 0, doc: /* Apply FUNCTION to each element of SEQUENCE, and concat the results as strings. In between each pair of results, stick in SEPARATOR. Thus, " " as SEPARATOR results in spaces between the values returned by FUNCTION. + SEQUENCE may be a list, a vector, a bool-vector, or a string. -SEPARATOR must be a string, a vector, or a list of characters. + +Optional argument SEPARATOR must be a string, a vector, or a list of +characters; nil stands for the empty string. + FUNCTION must be a function of one argument, and must return a value that is a sequence of characters: either a string, or a vector or list of numbers that are valid character codepoints. */) diff --git a/test/src/fns-tests.el b/test/src/fns-tests.el index 5759457209..2d641cc311 100644 --- a/test/src/fns-tests.el +++ b/test/src/fns-tests.el @@ -430,6 +430,23 @@ fns-tests-hash-buffer (buffer-hash)) (sha1 "foo")))) +(ert-deftest fns-tests-mapconcat () + (should (string= (mapconcat #'identity '()) "")) + (should (string= (mapconcat #'identity '("a" "b")) "ab")) + (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")) + ;; non-ASCII strings + (should (string= (mapconcat #'identity '("Ä" "ø" "☭" "தமிழ்") "_漢字_") + "Ä_漢字_ø_漢字_☭_漢字_தமிழ்")) + ;; vector + (should (string= (mapconcat #'identity ["a" "b"] "") "ab")) + ;; bool-vector + (should (string= (mapconcat #'identity [nil nil] "") "")) + (should-error (mapconcat #'identity [nil nil t]) + :type 'wrong-type-argument)) + (ert-deftest fns-tests-mapcan () (should-error (mapcan)) (should-error (mapcan #'identity)) -- 2.30.2 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* bug#50965: Allow nil third argument for mapconcat 2021-10-03 18:34 ` Stefan Kangas @ 2021-10-05 13:02 ` Eli Zaretskii 2021-10-05 13:39 ` Stefan Kangas 0 siblings, 1 reply; 8+ messages in thread From: Eli Zaretskii @ 2021-10-05 13:02 UTC (permalink / raw) To: Stefan Kangas; +Cc: larsi, 50965 > From: Stefan Kangas <stefan@marxist.se> > Date: Sun, 3 Oct 2021 18:34:31 +0000 > Cc: larsi@gnus.org, 50965@debbugs.gnu.org > > New patch attached. LGTM, thanks. ^ permalink raw reply [flat|nested] 8+ messages in thread
* bug#50965: Allow nil third argument for mapconcat 2021-10-05 13:02 ` Eli Zaretskii @ 2021-10-05 13:39 ` Stefan Kangas 0 siblings, 0 replies; 8+ messages in thread From: Stefan Kangas @ 2021-10-05 13:39 UTC (permalink / raw) To: Eli Zaretskii; +Cc: larsi, 50965 tags 50965 fixed close 50965 29.1 thanks Eli Zaretskii <eliz@gnu.org> writes: > LGTM, thanks. Thanks, pushed to master as commit d652efcd08. ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2021-10-05 13:39 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2021-10-02 13:22 bug#50965: Allow nil third argument for mapconcat Stefan Kangas 2021-10-02 13:50 ` Lars Ingebrigtsen 2021-10-02 20:12 ` Stefan Kangas 2021-10-02 23:03 ` Stefan Kangas 2021-10-03 5:56 ` Eli Zaretskii 2021-10-03 18:34 ` Stefan Kangas 2021-10-05 13:02 ` Eli Zaretskii 2021-10-05 13:39 ` Stefan Kangas
Code repositories for project(s) associated with this public inbox https://git.savannah.gnu.org/cgit/emacs.git This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).